Re: pack operation is thrashing my server

Nicolas Pitre [off-list ref] writes:

On Sat, 6 Sep 2008, Junio C Hamano wrote:

quoted

Linus Torvalds [off-list ref] writes:
...

quoted

Which it does - but the patch kind of violates that whole design.

Now, it so happens that things seem to work, probably because the zlib 
format does have enough synchronization in it to not try to continue past 
the end _anyway_, but I think this makes the patch be of debatable value.

I thought the fact we do check the status with Z_STREAM_END means that we
do already expect and rely on zlib to know where the end of input stream
is, and stop there (otherwise we say something fishy is going on and we
error out), and it was part of the design, not just "so happens" and "has
enough synch ... _anyway_".

If input zlib stream were corrupted and it detected the end of stream too
early, then check of "stream.total_out != size" would fail even though we
would see "st == Z_STREAM_END".  If input stream were corrupted and it
went past the end marker, we will read past the end and into some garbage
that is the in-pack header of the next object representation, but zlib
shouldn't go berserk even in that case, and would stop after filling the
slop you allocated in the buffer --- we would detect the situation from
stream.total_out != size and most likely st != Z_STREAM_END in such a
case.

Unless I'm missing something, I think your analysis is right and 
everything should be safe.

I obviously agree with you but what I forgot to mention in the above is
that we also make sure stream.avail_in is set not to overrun the end of
the current pack window (or the entire loose object data that is
mmapped).