Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0006441 [Squeak] Kernel minor always 04-23-07 09:33 11-18-08 20:11
Reporter johnpf View Status public  
Assigned To
Priority normal Resolution fixed  
Status closed   Product Version 3.10
Summary 0006441: [BUG] SmallInteger hex not understood
Description message hex is understood by Character and Float. The wiki page http://wiki.squeak.org/squeak/2137 [^] includes the code "i hex" where i is a SmallInteger.

$a hex.

is not understood though, as SmallInteger doesn't understand it.

A lazy fix is to add

SmallInteger>>hex
^self asFloat hex.

but this seems wrong, as any Integer should understand it, and possibly any Number, especially as if a Float understands it!

Tested in 3.9 and 3.10a (build 7081), fails in both.
Additional Information
Attached Files  Integer-hex.st [^] (168 bytes) 06-25-07 11:33
 Hex-Tests.st [^] (1,218 bytes) 06-26-07 00:48
 Integer-hex.st.2 [^] (485 bytes) 06-26-07 11:13

- Relationships
related to 0006339closed  Character>> hex method fails 

- Notes
(0010606 - 502 - 588 - 588 - 588 - 588 - 588)
johnpf
04-23-07 13:49

My lazy fix is almost certainly wrong. Character>>hex gives the error "SmallInteger does not understand hex" though, which makes me wonder how ZipArchive>>readFrom: work with a broken zipfile, as it (indirectly) uses it (via String>>asHex) when there's an error in the zip file's signature (I think).

Another place where it's called is in FixUnderscores>>Initialize (class method),
where there's:

self arrowChar asInteger hex

but "FixUnderscores arrowChar asInteger hex." errors as expected.
 
(0010608 - 91 - 142 - 142 - 142 - 142 - 142)
johnpf
04-23-07 15:33

Better Fix:

Integer>>hex
   ^self printStringBase: 16

Makes the examples above work.
 
(0010828 - 1022 - 1242 - 1242 - 1242 - 1242 - 1242)
wiz
06-25-07 02:57

HI johnpf,


Hmm. The real question IMHO is why does it fail in small integer.

$a isInteger is false.

So technically $a hex should not work better than

aNumber hex

The 3dot8 version of the image had a Integer>>hex

hex
    self deprecated: 'Use ', self printString, ' printStringHex or ', self printString, ' storeStringHex instead!'.
    ^ self storeStringBase: 16
------

It seems to me the problem is that a method was deprecated but not the message. That leaves the meaning of the message #hex incomplete.

Using the #printStringHex or #storeStringHex with characters doesn't work so the meaning of those messages are inconsistent also.

Since you've brought the issue up how about writing some tests to point this out to the fine folk who maintain the image?

Else we will just go thru this deprecate it; put it back cycle over and over again.

Cheers, Jerome Peace

Note a method is the implementation of a message in one class. A message is the selector its receiver and all arguments.
 
(0010829 - 2726 - 3272 - 3272 - 3272 - 3272 - 3272)
johnpf
06-25-07 11:26

To my way of thinking, a character is just a semantic interpretation of a byte-sized integer (the attitude of a C programmer). If you look at the class definition of Character, it has a single instance variable, which is indeed a positive integer less than 256.

The more I look at this, the more problems I find. Looking at the various methods involved, and what would seem to me to be 'sane' behaviour:

1) Senders of hex:
a)to a Character
String>>asHex - should produce a string where each char is replaced by two hex digits (hexits?)
b) to an Integer
Character>>hex - I'd expect a two digit string from 00 to FF
FixUnderscores>>initialize - contains "self arrowChar asInteger hex" ( should return '8F' )
MethodFinder>>initialize - hex is added as an 'Approved' method of Integer

2) Method Integer>>asHexDigit
    ^'0123456789ABCDEF' at: self+1

This plainly errors with a 'subscript is out of bounds for any int < 0 or > 15. I think it should send an error: message.

All senders of this take great care to only have receivers 0 to 15, so they have to care about the internals of the method - this seems wrong somehow!

3) Integer>>printStringHex
    ^self printStringBase: 16
There are 15 senders of this.

This seems fine until you see how it's used in
a)Color>>printHTMLString (see bug 0005241) (I have to wonder how many other bugs there are where a 2 digit result has been assumed)

yet in
b) Color>>hex: aFloat
there is a check to make sure we get a 2 digit result. It is only sent by Color>>asHTMLColor. The code is almost what is needed elsewhere though.

FIX: (closes 0006441, 0006339, 0000987 and 0005241)
(Option 1 - don't deprecate Integer>>hex - it seems to me to be a reasonable thing to ask for)
Integer>>hex
"receiver is in range 0 to 255"
^self printStringBase: 16 length: 2 padded: true

(Option 2)
Change all the methods in (1) above to use
printStringBase: 16 length: 2 padded: true
instead of
hex

IF Option 1 is ued we then need
Color>>printHtmlString
"answer a string whose characters are the html representation of the receiver"
    ^ (self red * 255) asInteger hex, (self green * 255) asInteger hex, (self blue * 255) asInteger hex

IF we use Option 2 then we need
Color>>printHtmlString
"answer a string whose characters are the html representation of the receiver"
    ^ (self red * 255) asInteger printStringBase: 16 length: 2 padded: true, (self green * 255) asInteger printStringBase: 16 length: 2 padded: true, (self blue * 255) asInteger printStringBase: 16 length: 2 padded: true

which is ugly.

In either case the other 14 senders of Integer>>printStringHex need to be checked whether they need to return padded 2 digit strings or not.
 
(0010830 - 49 - 55 - 55 - 55 - 55 - 55)
edgardec
06-25-07 12:09

This go to 7114Integer-hex.cs .
Very thanks John
 
(0010832 - 675 - 795 - 795 - 795 - 795 - 795)
wiz
06-25-07 18:29

Hi John. Hi Edgar

I am glad to see it gets fixed in 3dot10

It really, really, needs to have tests with it

simple things

self shouldnt: [ $a hex ] raise: Error

with a slew of examples.

This would be a good exercise for John because he has given a lot of thought to the design.

The tests are a living design document and a good way to express all you wrote in your last note.

The tests can exist before the code is implemented (red flag tests) and work (green flag ) once all repairs are made.

The true problem with this bug is the deprecation happened inconsistenly because the tests were absent.

Yours in curiosity and service, --Jerome Peace
 
(0010833 - 98 - 98 - 98 - 98 - 98 - 98)
johnpf
06-26-07 00:51

Added Tests (Hex-Tests.st). Currently 4 fails/errors, both fix options above result in 4 passes.
 
(0010834 - 55 - 55 - 55 - 55 - 55 - 55)
johnpf
06-26-07 01:00

oops, wrong file - the one uploaded only tests Option 1
 
(0010835 - 228 - 282 - 282 - 282 - 282 - 282)
edgardec
06-26-07 10:16

I see in old Squeak result _ 15 asInteger hex is "OF" , but in Squeak 3.10 is "F"

The Hex-Tests.st gives fail on this, but IMHO the print should give "OF" as in older Squeak.

Could fix this also ? Very thanks for your work
 
(0010836 - 239 - 239 - 239 - 239 - 239 - 239)
johnpf
06-26-07 11:12

Edgar, I didn't check your Integer-hex.st before. Your implementation is certainly mathematically valid. I believe that most users would expect one digit per nibble though. I've uploaded a revised Integer-hex.st with a detailed comment.
 
(0010837 - 151 - 151 - 151 - 151 - 151 - 151)
johnpf
06-26-07 11:16

I had to butcher the filename, as I don't seem to have rights to delete the previous version. I don't particularly want that level of privilege either.
 
(0010838 - 299 - 329 - 329 - 329 - 329 - 329)
edgardec
06-26-07 11:31

John
Very good. Now we have a better implementation and test for it.
The previous implementation is on older Squeak and is not mine.
Don't worry of deleting files , as I think is good for see how some improves.
I don a new updates with the test and the new implementation.
Again , very thanks !
 
(0010839 - 134 - 158 - 158 - 158 - 158 - 158)
wiz
06-27-07 05:12

HI John,

Thanks for taking on the challenge and following thru to make the tests.

Yours in curiosity and service, --Jerome Peace
 
(0010853 - 757 - 925 - 925 - 925 - 925 - 925)
wiz
07-03-07 02:38

Now that 7121 is up on the ftp site I've noticed that the replacement for Number>>hex does not do what it used to do in 6665 (i.e. squeak 3.8 final.

the difference is that number hex would produce the number form

i.e. 15 hex 16rF in 6665
but 15 hex '0F' in 7121

The printit I used was:

(0 to: 255 ) collect: [ :each |
each hex ] .

So the question is

1) does this matter to anyone?

And
2) how do we find them to find out?

3) does this break any code of importance?

4) does it further fork us from the squeakland version of squeak?.

5) does if further fork us from the tweak version of squeak?

6) does it further fork us from the smallland version of squeak?

Yours in curiosity and service, -Jerome Peace
 
(0010856 - 1509 - 2109 - 2109 - 2109 - 2109 - 2109)
johnpf
07-03-07 12:55

The only place I would most 'expect' breakage would be when a ZipArchive has a bad signature:

({CentralDirectoryFileHeaderSignature. LocalFileHeaderSignature. EndOfCentralDirectorySignature} includes: signatureData)
        ifFalse: [^ self error: 'bad signature ' , signatureData asString asHex , ' at position ' , (stream position - 4) asString].

So the error includes a string of the found signature. Looking at ZipFileConstants, we see, for example,

LocalFileHeaderSignature := ByteArray
                with: 80
                with: 75
                with: 3
                with: 4.


My implementation gives '504B0304', the older one '504B34'. So the old implementation didn't accurately map to a bitstream however you look at it!

So debugging a bad zip file now will more descriptively show a signature that's wrong (always shows a string of length 8 for 4 bytes, instead of a string of length between 4 and 8).

Color>>printHtmlString is currently broken anyway, my proposed fix depends on Integer>>hex

(3) is answered above
(1) obviously includes me

I have only considered Integer>>hex - float has a perfectly functional method already, and I struggle to make sense of a hex that Fraction understands. I have no idea why anyone would expect Number to understand it (except as an abstract method) as you would have to coerce a 'number' to a float, int or bytearray to do anything meaningful with it.

The answers to your other questions are possibly found by

0) Why was Integer>>hex deprecated in the first place?
 
(0010865 - 213 - 213 - 341 - 341 - 341 - 341)
matthewf
07-09-07 06:25

I cannot add a relationship to the bug database, but there is much more early discussion of this bug at http://bugs.squeak.org/view.php?id=6339 [^] , although they do not specify the reason for deprecating this method
 
(0010866 - 84 - 90 - 218 - 218 - 218 - 218)
matthewf
07-09-07 07:05

Opened a bug on sister methed Integer>>hex8: http://bugs.squeak.org/view.php?id=6558 [^]
 
(0012781 - 62 - 62 - 62 - 62 - 62 - 62)
KenCausey
11-18-08 20:11

Harvested in update 7114 and 7119 and released in Squeak 3.10.
 

- Issue History
Date Modified Username Field Change
04-23-07 09:33 johnpf New Issue
04-23-07 13:49 johnpf Note Added: 0010606
04-23-07 15:33 johnpf Note Added: 0010608
06-25-07 02:57 wiz Note Added: 0010828
06-25-07 11:26 johnpf Note Added: 0010829
06-25-07 11:26 johnpf Issue Monitored: johnpf
06-25-07 11:33 edgardec File Added: Integer-hex.st
06-25-07 12:09 edgardec Note Added: 0010830
06-25-07 18:29 wiz Note Added: 0010832
06-25-07 18:30 wiz Status new => feedback
06-25-07 18:30 wiz Resolution open => reopened
06-25-07 18:30 wiz Fixed in Version  => 3.10
06-26-07 00:48 johnpf File Added: Hex-Tests.st
06-26-07 00:51 johnpf Note Added: 0010833
06-26-07 01:00 johnpf Note Added: 0010834
06-26-07 10:16 edgardec Note Added: 0010835
06-26-07 11:12 johnpf Note Added: 0010836
06-26-07 11:13 johnpf File Added: Integer-hex.st.2
06-26-07 11:16 johnpf Note Added: 0010837
06-26-07 11:31 edgardec Note Added: 0010838
06-27-07 05:12 wiz Note Added: 0010839
07-03-07 02:38 wiz Note Added: 0010853
07-03-07 12:55 johnpf Note Added: 0010856
07-09-07 06:25 matthewf Note Added: 0010865
07-09-07 07:05 matthewf Note Added: 0010866
07-17-07 03:35 wiz Relationship added related to 0006339
11-18-08 20:11 KenCausey Status feedback => closed
11-18-08 20:11 KenCausey Note Added: 0012781
11-18-08 20:11 KenCausey Resolution reopened => fixed


Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
121 total queries executed.
69 unique queries executed.
Powered by Mantis Bugtracker