[Babel-users] babeld: memory leak?

[Gabriel Kerneis]

Hi,

On Fri, Jul 15, 2011 at 12:50:30PM +0200, Matthieu Boutier wrote:
> config.c, 718-729:
> we lost all network_confs, but:

Well, not really lost, but linked from the global networks list.

> network.c, 59:
> we catch them, but if we reach line 70, we really lost the network_conf, I
> think, no?

Anyway, this case should never happen when conf != NULL because add_nconf
already takes care of merging network configurations for the same interface
(config.c:488).

But it might happen when conf is NULL, though: when an interface is duplicated
on the command line (babeld.c:350, see below), or when it appears both on the
command line and in the config file or in a -C statement.

> (75 too, but at least an error occurs, and then program will properly end)

When malloc fails, it is probably too late to worry about memory leaks ;-)

> NB: tests of lines 63, 64 are never usefull… why having create it ?

http://en.wikipedia.org/wiki/Defensive_programming

Also notice that line 63 is indeed useful, because of:
> babeld.c:350:        vrc = add_network(argv[i], NULL);

To get a really defensive function avoiding space leaks, you could apply the
following patch:

diff -rN -u old-babeld/network.c new-babeld/network.c
--- old-babeld/network.c    2011-07-15 14:20:34.000000000 +0200
+++ new-babeld/network.c    2011-07-15 14:20:35.000000000 +0200
@@ -29,6 +29,7 @@
 #include <netinet/in.h>
 #include <net/if.h>
 #include <arpa/inet.h>
+#include <assert.h>
 
 #include "babeld.h"
 #include "util.h"
@@ -66,8 +67,10 @@
     }
 
     FOR_ALL_NETS(net) {
-        if(strcmp(net->ifname, ifname) == 0)
+        if(strcmp(net->ifname, ifname) == 0) {
+            assert(!conf);
             return net;
+        }
     }
 
     net = malloc(sizeof(struct network));

It might be overkill, though.

Best,
-- 
Gabriel