[squid-dev] [PATCH] Refactor htcpDetail into MEMPROXY_CLASS

[Kinkie]

My point.
At the same time, I'd like others to chime in as well. It is a tiny change
(turn an assert into an if) but the implications are quite profound.
I don't expect it'll fault right away - the assert is not firing right now
so clients obviously take care of checking.
On Sep 29, 2015 11:34 AM, "Amos Jeffries" <squid3 at treenet.co.nz> wrote:

> On 29/09/2015 5:59 p.m., Kinkie wrote:
> > On Mon, Sep 28, 2015 at 10:10 AM, Amos Jeffries <squid3 at treenet.co.nz>
> wrote:
> >> On 28/09/2015 7:28 p.m., Kinkie wrote:
> >>> Whoops, sorry.
> >>> The patch includes spurious changes.
> >>> Updated patch attached.
> >>>
> >>
> >> +1.  Though please fix the indentation on "delete d;" in chunk @1100.
> >> Which should be 4 not 8 SP.
> >
> > Huh?
> > code is:
> >     if (d)
> >         delete d;
> >
> > Indentation looks legitimate to me..
> >
> > BTW: there is a substantial difference in the memproxy_class behavior
> > wrt standard "delete": AFAICS, "delete nullptr" is a perfectly
> > legitimate statement by the standard and it is a NOP. When going
> > through mempools, it reaches MemImplementingAllocator::freeOne which
> > asserts. would it make sense to NOP ourselves?
> >
>
> Yes. That is a bug.
>
> delete should always be the same API semantics and promises.
>
> So no need for the if(d) to exist around a delete, and if that causes
> crashes/errors inside our custom delete operators they are what is borked.
>
> Amos
>
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150929/1fb5eb4c/attachment.html>