Mantis - Squeak
Viewing Issue Advanced Details
6441 Kernel minor always 04-23-07 09:33 11-18-08 20:11
johnpf  
 
normal  
closed 3.10  
fixed  
none    
none 3.10  
0006441: [BUG] SmallInteger hex not understood
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.
related to 0006339closed  Character>> hex method fails 
 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

Notes
(0010606)
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)
johnpf   
04-23-07 15:33   
Better Fix:

Integer>>hex
   ^self printStringBase: 16

Makes the examples above work.
(0010828)
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)
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)
edgardec   
06-25-07 12:09   
This go to 7114Integer-hex.cs .
Very thanks John
(0010832)
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)
johnpf   
06-26-07 00:51   
Added Tests (Hex-Tests.st). Currently 4 fails/errors, both fix options above result in 4 passes.
(0010834)
johnpf   
06-26-07 01:00   
oops, wrong file - the one uploaded only tests Option 1
(0010835)
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)
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)
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)
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)
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)
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)
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)
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)
matthewf   
07-09-07 07:05   
Opened a bug on sister methed Integer>>hex8: http://bugs.squeak.org/view.php?id=6558 [^]
(0012781)
KenCausey   
11-18-08 20:11   
Harvested in update 7114 and 7119 and released in Squeak 3.10.