Keresztfalvi Gabor <[EMAIL PROTECTED]> writes:
> (gdb) print rc
> $1 = -5 (which is Z_BUF_ERROR)
> (gdb) print self->z
> $2 = {next_in = 0x812149b "", avail_in = 0, total_in = 56169,
> next_out = 0x810fa34 "", avail_out = 1, total_out = 66138, msg = 0x0,
> state = 0x80ad534, zalloc = 0x8061758 <zlib_alloc>,
> zfree = 0x8061770 <zlib_free>, opaque = 0x80497ac, data_type = 0,
> adler = 368617483, reserved = 0}
> (gdb) print self->f
> $4 = (int (*)(struct z_stream_s *, int)) 0x80493ac <inflate>
Thanks for looking into this.
> zlib.h sais:
> inflate() returns Z_OK if some progress has been made (more input processed
> or more output produced), Z_STREAM_END if the end of the compressed data has
> been reached and all uncompressed output has been produced, Z_NEED_DICT if a
> preset dictionary is needed at this point, Z_DATA_ERROR if the input data was
> corrupted (input stream not conforming to the zlib format or incorrect
> adler32 checksum), Z_STREAM_ERROR if the stream structure was inconsistent
> (for example if next_in or next_out was NULL), Z_MEM_ERROR if there was not
> enough memory, Z_BUF_ERROR if no progress is possible or if there was not
> enough room in the output buffer when Z_FINISH is used. In the Z_DATA_ERROR
> case, the application may then call inflateSync to look for a good
> compression block.
>
> I've written a fprintf before doing self->f(...) and I got this in the
> client at the critical point:
> *** avail_in == 32783 avail_out == 32768
> do_zlib: Both avail_in and avail_out are zero.
> *** avail_in == 0 avail_out == 1
> do_zlib: deflate() or inflate() failed: No error(?)
> Here avail_out still equals to 1...
> If I write self->max+1 in estimate_size, then everything works fine...
> Seems, that the whole packet is inflated, which just fits the output
> buffer, so both avail_in and avail_out will be 0. So we grow the output
> buffer with 1 (as the limit sais this), and call inflate() again. And
> inflate() thinks badly from that it has only one free byte, that it can't
> progress, before it thinks about wheteher it really should progress...
Hmm. I think the reason zlib can't make any progress is that it has
actually processed all input (and avail_in is zero). But we still has
to call it, because there's no other way to know for sure.
Changing the estimate logic to include an extra byte doesn't really
fix the bug; it shouldn't matter (except for efficiency) if the extra
byte is allocated at the start or later. Please try the (untested)
patch below.
> P.s.: BTW why doesn't the protocol itself transfers the original size of
> the compressed data? Then we souldn't do guesses about how big the data
> will grow...
I've been thinking about that as well. We could create a new
"zlib-lsh" compression method to do something like that. Perhaps the
length field should compressed too? However, this will only help
inflating; when deflating we still have to guess and adapt to whatever
the compression ratio turns out to be.
=== cd /home/nisse/hack/lsh/src/
=== cvs -d /lysator/cvsroot/ diff -u -a zlib.c
Index: zlib.c
===================================================================
RCS file: /lysator/cvsroot/nisse/lsh/src/zlib.c,v
retrieving revision 1.25
diff -u -a -r1.25 zlib.c
--- zlib.c 2000/02/15 19:03:51 1.25
+++ zlib.c 2000/02/17 09:47:40
@@ -160,11 +160,10 @@
return free ? packet : lsh_string_dup(packet);
}
- estimate = estimate_size(self->rate, packet->length, self->max);
+ estimate = estimate_size(self->rate, packet->length, limit);
debug("do_zlib: estimate: %i\n", estimate);
- string_buffer_init(&buffer,
- estimate_size(self->rate, packet->length, self->max));
+ string_buffer_init(&buffer, estimate);
limit -= buffer.partial->length;
@@ -178,18 +177,28 @@
self->z.next_out = buffer.current;
self->z.avail_out = buffer.left;
+ assert(self->z.avail_out);
+
rc = self->f(&self->z, Z_SYNC_FLUSH);
- if (rc != Z_OK)
+ switch (rc)
{
+ case Z_BUF_ERROR:
+ werror("zlib.c: Z_BUF_ERROR (probably harmless),\n"
+ " avail_in = %i, avail_out = %i\n",
+ self->z.avail_in, self->z.avail_out);
+ /* Fall through */
+ case Z_OK:
+ break;
+ default:
werror("do_zlib: deflate() or inflate() failed: %z\n",
self->z.msg ? self->z.msg : "No error(?)");
if (free)
lsh_string_free(packet);
-
+
return NULL;
}
-
+
/* NOTE: It's not enough to check that avail_in is zero to
* determine that all data have been flushed. avail_in == 0 and
* avail_out > 0 implies that all data has been flushed, but if
