On Fri, 11 Apr 2003 Daniel Stenberg wrote:

> On Fri, 11 Apr 2003 niclists_at_nichines.com wrote:
>
> > >Whoa. This is next to unbelievable. Dan Fandrich posted a patch doing
> > >this, and he beat you with ~11 hours! ;-)
> >
> > Damn! I searched around to see if it had been mentioned, I couldn't find
> > anything.
>
> No wonder, Dan's mail was still warm when your mail arrived!
>
> > conn->keep.str needs to be updated to point to the chunk data. Is it safe
> > to just update it like this? It seems okay when I test it against
> > http://sourceforge.net/
> >
> > --- http_chunks.c~1 2003-04-11 09:49:21.000000000 +0100
> > +++ http_chunks.c 2003-04-11 15:52:36.000000000 +0100
> > @@ -186,10 +186,16 @@
> > break;
> >
> > case DEFLATE:
> > + /* update conn->keep.str to point to the chunk data. */
> > + conn->keep.str = datap;
> > +
> > result = Curl_unencode_deflate_write(conn->data, &conn->keep,
> > piece);
> > break;
> >
> > case GZIP:
> > + /* update conn->keep.str to point to the chunk data. */
> > + conn->keep.str = datap;
> > +
> > result = Curl_unencode_gzip_write(conn->data, &conn->keep,
> piece);
> > break;
>
> Ah, now I can see how this is a bug, yes.
>
> I think I'd prefer to pass the 'datap' as an input argument to these
> functions instead, as I think it makes it easier to read and understand what
> the pointer comes from and what it is used for. You agree?

Actually, I'm not sure I do *8-) from what I could see "str" is modified
in other places in a similar manner (transfer.c). It's really no different
to the case where your buffer has half headers and half body and you change
"str" to point to the next thing in the buffer to be dealt with. Also
adding another argument to the function would mean changing all the
references to k->str in the unencode functions.

>
> > At first glance I think it is one point each *8-) Dan's handles extended
> > headers but mine checks the CRC, which I don't think would be a big
problem
> > to add to Dan's code. I'll look closer at Dan's stuff and see how it
> > compares. It's good to see that he had the same problem of having to cope
> > with fragmentation when dealing with the GZIP header.
> >
> > To test the fragmentation handling I amended transfer.c and added a loop
> > around the call to Curl_unencode_deflate_write() to pass data in character
> > by character. By doing this I discovered that the inflate can return with
> > (DSIZ - z->avail_out) == 0 and Curl_client_write() didn't like a length of
> > 0 so I would recommend the following...
>
> [patch]
>
> So this patch improves the current behavior? Does it make sense then if I
> apply and commmit this patch now, and give you some time to get back with a
> separate one that adds your additional stuff ontop of Dan's previous patch?
>

By all means commit the patch. I don't believe it will do any harm.

I didn't have time to follow it through but it appeared to me that
Curl_client_write() did something funny if it was passed a length of zero.
This change will prevent it being called in this circumstance. I think it
can happen if the inflate() function has been handling control data and it
did not actually produce any normal output during the pass.

> Many thanks!

Regards,

Nic

-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger
for complex code. Debugging C/C++ programs can leave you feeling lost and
disoriented. TotalView can help you find your way. Available on major UNIX
and Linux platforms. Try it free. www.etnus.com
Received on 2003-04-11