patuww/zlib-win64

Code may compile without warnings but doesn't work!!

Closed this issue · 1 comments

I downloaded this project because I wanted a zlib under 64 bit on windows.

My recommendation to anyone else thinking of using this is don't.


Unfortunately the changes in the code to remove warnings are simply incorrect.

The clearest example of this is to look at the difference between inflate.c in 
this version and the downstream 1.2.7 inflate.c

this version


/* Get a byte of input into the bit accumulator, or return from inflate()
   if there is no input available. */
#define PULLBYTE() \
    if (have == 0) goto inf_leave; \
    have--; \
    hold += (unsigned long)(*next++) << bits; \
    bits += 8;

/* Assure that there are at least n bits in the bit accumulator.  If there is
   not enough available input to do that, then return from inflate(). */
#define NEEDBITS(n) \
    while (bits < (unsigned)(n)) \
        PULLBYTE();


Equivalent Source from original 1.2.7

/* Get a byte of input into the bit accumulator, or return from inflate()
   if there is no input available. */
#define PULLBYTE() \
    do { \
        if (have == 0) goto inf_leave; \
        have--; \
        hold += (unsigned long)(*next++) << bits; \
        bits += 8; \
    } while (0)

/* Assure that there are at least n bits in the bit accumulator.  If there is
   not enough available input to do that, then return from inflate(). */
#define NEEDBITS(n) \
    do { \
        while (bits < (unsigned)(n)) \
            PULLBYTE(); \
    } while (0)

As can be seen the do { } while ( 0 ) have been removed (probably because they 
warn about conditional is constant).

They have been removed incorrectly - since the macro expansion for say 
PULLBYTE() is no longer a single statement.

This means that the first case of this macro being used in inflate.c

NEEDBITS(16);

is actually translated by the macro expansion to the following fragment

while (bits < (unsigned)(n))
    if (have == 0) goto inf_leave;

which is an infinite loop.

This leads me to suspect that this code has not been tested at all since any 
call to inflate() will end up in an infinite loop.

I haven't gone any further through this code having seen this - so there may 
well be other equally as wrong changes.




Original issue reported on code.google.com by Alex.Pe...@smartlogic.com on 13 May 2014 at 2:43

Multiple fixes made to ensure correct functionality including corrections to 
infinite loops introduced by the previous version.

Original comment by han...@gmail.com on 31 May 2014 at 2:29

  • Changed state: Fixed