RE: squid 2.6-rproxy: locrewrite.c bug

Whew, the whole callback architecture is a bit of a learning curve for
me. I studied some other examples from the code, and came up with:

57: void *cbdata;
65: if (cbdataReferenceValidDone(r->data,&cbdata))
66: r->handler(cbdata, reply);
67: locationRewriteStateFree(r);

Is this it? (patch attached)

Thanks,
Jeff

--
NearlyFreeSpeech.NET 
$1/1GB Web Hosting, no minimums, no monthly fees, no kidding.
http://www.nearlyfreespeech.net/
> -----Original Message-----
> From: Henrik Nordstrom [mailto:hno@squid-cache.org] 
> Sent: Tuesday, June 18, 2002 6:35 PM
> To: Jeffrey D. Wheelhouse
> Cc: 'Squid Developers Mailinglist'
> Subject: Re: squid 2.6-rproxy: locrewrite.c bug
> 
> 
> For this kind of situations there is the cbdataReferenceDoneValid (or 
> is it cbdataReferenceValidDone, I never remeber which). See the 
> programmers guide for relatively detailed instructions and a couple 
> of examples. There is also numerous examples in other parts of the 
> code.
> 
> Regards
> Henrik
> 
> 
> 
> On Wednesday 19 June 2002 00.52, Jeffrey D. Wheelhouse wrote:
> > I've run into a snag with the new-style cbdata handling in
> > locrewrite.c.
> >
> > The pre-patch locationRewriteHandleReply:
> > 65:    valid = cbdataValid(r->data);
> > 66:    cbdataUnlock(r->data);
> > 67:    if (valid)
> > 68:     r->handler(r->data, reply);
> > 69:    locationRewriteStateFree(r);
> >
> > The updated code:
> >
> > 65:    valid = cbdataReferenceValid(r->data);
> > 66:    cbdataReferenceDone(r->data);
> > 67:    if (valid)
> > 68:     r->handler(r->data, reply);
> > 69:    locationRewriteStateFree(r);
> >
> > This promptly leads to cores, because cbdataReferenceDone wipes out
> > r->data.  Duh.  I feel very silly for not realizing this the first
> > time.
> >
> > I am testing a patch (attached) that yields:
> >
> > 64:    if (cbdataReferenceValid(r->data))
> > 65:     r->handler(r->data, reply);
> > 66:    cbdataReferenceDone(r->data);
> > 67:    locationRewriteStateFree(r);
> >
> > However, the whole point of "valid" seems to be to do the
> > unreference before calling r->handler, and I'm not sure why.  So
> > this may not be the correct fix.
> >
> > This patch should also be tabbed correctly (finally).
> >
> > I also have the read_ahead_gap config patch done, but nontrivial
> > testing of that has been gated by this issue until now.  If this is
> > a good fix, the read_ahead_gap patch will be done testing Thursday.
> >
> > Thanks,
> > Jeff
>