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

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Wed, 04 Jan 2012 19:05:44 +1300

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.

Amos
Received on Wed Jan 04 2012 - 06:06:06 MST

This archive was generated by hypermail 2.2.0 : Wed Jan 04 2012 - 12:00:11 MST