Re: [Patch] 2.5.70-bk11 zlib merge #4 pure magic

From: Jörn Engel (joern@wohnheim.fh-wedel.de)
Date: Sat Jun 07 2003 - 04:20:27 EST


On Fri, 6 June 2003 22:36:07 +0200, Bartlomiej Zolnierkiewicz wrote:
> On Fri, 6 Jun 2003, [iso-8859-1] Jörn Engel wrote:
>
> > This one is pure magic, really. No comment and inspection of the code
> > doesn't show much either. But judging from the other changes, this
> > should also fix a real problem, at least a theoretical one.
>
> Eee, no magic here :-).
>
> from 1.1.4 ChangeLog:
> "- force windowBits > 8 to avoid a bug in the encoder for a window size
> of 256 bytes. (A complete fix will be available in 1.1.5)."

So there IS a ChangeLog. :)

> I guess complete fix is here:
> http://www.cs.toronto.edu/~cosmin/pngtech/zlib-256win-bug.html

> --- deflate.c Thu Jul 09 18:06:12 1998
> +++ deflate.c.fixed Thu Apr 12 04:02:36 2001
> @@ -242,7 +242,7 @@
> windowBits = -windowBits;
> }
> if (memLevel < 1 || memLevel > MAX_MEM_LEVEL || method !=
> Z_DEFLATED ||
> - windowBits < 8 || windowBits > 15 || level < 0 || level > 9 ||
> + windowBits > 15 || level < 0 || level > 9 ||
> strategy < 0 || strategy > Z_HUFFMAN_ONLY) {
> return Z_STREAM_ERROR;
> }

Completely removes the check.

> @@ -252,7 +252,11 @@
> s->strm = strm;
>
> s->noheader = noheader;
> - s->w_bits = windowBits;
> +#if MIN_LOOKAHEAD < 256
> + s->w_bits = (windowBits >= 8) ? windowBits : 8;
> +#else
> + s->w_bits = (windowBits >= 9) ? windowBits : 9;
> +#endif
> s->w_size = 1 << s->w_bits;
> s->w_mask = s->w_size - 1;

Now we don't check and inform the user anymore, instead we change the
value internally. So now overly smart users can set the windowBits to
a stupid value and we correct their mistake instead of telling them.
Not my preferred style.

> @@ -460,7 +464,12 @@
> /* Write the zlib header */
> if (s->status == INIT_STATE) {
>
> +#if MIN_LOOKAHEAD < 256
> uInt header = (Z_DEFLATED + ((s->w_bits-8)<<4)) << 8;
> +#else
> + uInt optimized_cinfo = (s->w_bits > 9) ? s->w_bits - 8 : 0;
> + uInt header = (Z_DEFLATED + (optimized_cinfo<<4)) << 8;
> +#endif
> uInt level_flags = (s->level-1) >> 1;
>
> if (level_flags > 3) level_flags = 3;

This changes one corner case where windowBits == 9. Even if the fix
is correct (too lazy to check), it costs us four new conditionals, of
which two were moved into the preprocessor. And the gain over patch
#4 is zero, since we waste the memory anyway, where windowBits is
too small for decent compression or not. Rejected.

Still, thanks for the two pointers, Bartlomiej! Not quite as magic
anymore. :)

Jörn

-- 
With a PC, I always felt limited by the software available. On Unix, 
I am limited only by my knowledge.
-- Peter J. Schoenster
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sat Jun 07 2003 - 22:00:32 EST