Fwd: [squeak-dev] A nitpick

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

Fwd: [squeak-dev] A nitpick

Eliot Miranda-2
See http://source.squeak.org/trunk/Compression/Compression-eem.41

---------- Forwarded message ----------
From: Yoshiki Ohshima <[hidden email]>
Date: Tue, May 6, 2014 at 2:09 PM
Subject: [squeak-dev] A nitpick
To: The general-purpose Squeak developers list <[hidden email]>


This can be a nitpick of the year, but there is a bug in
InflateStream>>decompressBlock:with:

There is a part that reads (toward the end):

------------------------
collection
        replaceFrom: readLimit+1
        to: readLimit + length + 1
        with: collection
        startingAt: readLimit - distance + 1.
------------------------

but here we want to replace "length" bytes in collection, as opposed
to length + 1 bytes, so it has to say:

------------------------
collection
        replaceFrom: readLimit+1
        to: readLimit + length
        with: collection
        startingAt: readLimit - distance + 1.
------------------------

(So is the non-primitive version in FastInflateStream.)

The reason it does not matter is that replaceFrom:to:with:startingAt:
does not care about the source and destination being the same and
overlap between them.  Even when the above bug puts one extra bytes in
collection temporarily, the extra byte will be wiped out in the next
iteration of the loop surrounding it.  However, if you try to port
this to a language that does the "right thing" when there is an
overlap, it manifests as a bug.

-- Yoshiki

P.S.
It was a "rare" delight to point out a bug in Andreas' code^^; We miss
you.




--
best,
Eliot