Re: [PATCH] StatHist refactoring interim merge, proposed fix for bug 3381

On Wed, Jan 4, 2012 at 7:05 AM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> On 4/01/2012 11:42 a.m., Kinkie wrote:
>>>>
>>>> At the cost of a bit of further creep-in, done. Here's a revised
>>>> patch, which on top of the previous one, also:
>>>> - actually uses the unit test
>>>> - delivers a complete default constructor for StatHist
>>>> - adds equality test for StatHist
>>>> - adds more stubs to the collection (stub_mem and stub_stmem) and uses
>>>> them
>>>
>>>
>>> Will review later.
>>
>>
>
> in Makefile.am:
> * tests_testStatHist_SOURCES - alphabetical please. The linker does not care
> about order in *_SOURCES and sorted its far easier to find duplicate
> linkages.
> * tests_testStatHist_LDADD -  $(COMPAT_LIB) must be last on the list after
> all the convenience libraries, (ie libmiscutil.la). Only system libraries
> may go after it.
>
> in StatHist.h
> * operator=()  - xfree() can take a NULL ptr safely.
> * destructor body can be replaced with safe_free(bins); or probably even
> just xfree(bins); now that there is a proper constructor.
>
> in stub_mem.cc
> * compat/xalloc.h does not need including directly.
> * please include protos.h instead of typedefs.h, with the comment "mem*()
> definitions still in protos.h" (on a separate line to teh #include).
>  - when the protos are moved whatever header they move to will have to take
> care of locating FREE as well, just like protos.h does now.

Hi,
  v4 of the patch attached. It covers all of the above, plus it
changes an assert to a condition in to cover a shutdown race in
StatHist::count().

-- 
    /kinkie