Re: [PATCH] Fix memory leaks in ident.cc

On 25/12/2012 7:24 p.m., Mike Mitchell wrote:
> Here is a revised patch for the memory leak in Ident.cc
> This version frees both the IdentStateData structure and a Connection
> structure.
> The Close_() definition now has the "static" keyword.
>
> Mike Mitchell
>
> ------------------------------------------------------------------------
> *From:* Mike Mitchell
> *Sent:* Friday, December 21, 2012 1:06 PM
> *To:* squid-dev_at_squid-cache.org
> *Subject:* [PATCH] Fix memory leaks in ident.cc
>
> The attached patch plugs a memory leak in ident.cc.
> The IdentStateData structure was not freed if the ident request failed.
>
> Mike Mitchell

Thank you. BUT... this adds a code desing regression in the form of an
Ident::Close_ static destructor function.

Please make that a cleanup member method and simply call it when
appropriate. Including the state destructor.

PS. IdentStateData is long overdue for conversion from explicit
cbdataAlloc/cbdataFree memory management to use of the CBDATA_CLASS2()
constructs. Would you mind doing that conversion as part of this cleanup
since you can obviously test IDENT better than any of us has been able to?
The steps are simple:
* add CBDATA_CLASS2(IdentStateData) to the *end* of the IdentStateData
class definition
* add CBDATA_INIT(IdentStateData) to the top of the .cc file
* replace cbdataAlloc/cbdataFree of that class with new/delete.
* test that is still works without any new bugs (use of memset on the
objects is the main problem found here).

Amos
Received on Thu Dec 27 2012 - 01:00:54 MST