See http://source.squeak.org/trunk/Compression/Compression-eem.41 ---------- Forwarded message ---------- From: Yoshiki Ohshima <Yoshiki.Ohshima@acm.org> Date: Tue, May 6, 2014 at 2:09 PM Subject: [squeak-dev] A nitpick To: The general-purpose Squeak developers list < squeak-dev@lists.squeakfoundation.org> 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