Thanks Yoshiki,

    I'm committing this now


On Tue, May 6, 2014 at 2:09 PM, Yoshiki Ohshima <Yoshiki.Ohshima at acm.org>wrote:

> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20140506/8f0b5066/attachment.htm