From: Roman Hoog Antink <rha@op...> - 2011-11-07 00:56:46
I looked through your first bunch of patches.
Nothing to say.
Chunk 2 white space error, see below (p3).
Also chunk 2: removal of rmconf from union looks like cleanup work, not
related to the task of p2. Move it to another patch please.
"@@ -63,27 +63,56 @@": this chunk and another (p3?) introduces comments
about initialisation that are obvious. IMHO comments should be removed.
Again "@@ -63,27 +63,56 @@": remove WSC comment
"@@ -603,6 +603,9 @@": duprmconf_shallow was written by me, so on a
first glance I suspect your change introduces either a double free or a
memleak. But this has to be confirmed by valgrind (or similar method),
using inherited remote confs and reload, just like all the other spots
touched by you for memory management improvements.
I know, you thought it all through, but I don't have the time to wind my
brain again around these data structures. A complete verification with
valgrind won't be feasible (too many execution paths). I'm not sure if
I'll find the time to run a couple of tests myself, once these patches
become formally correct.
First chunk of p3 (patch three) is a cleanup, which is caused by p2.
Please cleanup yourself :-).
The directory structure of the patches is good (patch -p1 conform):
Also good: the custom #ifdef statements marking your doings are gone.
P3 chunk 6 "@@ -521,16 +569,43 @@": at the end there is white space
cleanup, not related to your task.
Same goes for "@@ -1653,7 +1728,6 @@".
Fishy: @@ -86,7 +86,7 @@ (localconf.c)