Hi,

On Fri, Nov 06, 2015 at 08:42:39AM +0100, Steffan Karger wrote:
> init_route() can allocate memory in netlist, but fail in many more ways
> than just fail to allocate.  Thus, always check and clean up netlist if
> needed, instead of just when init_route() succeeds.
> 
> This fix is for master only.  The release/2.3 branch cleans up netlist
> immediately, and needs a different patch for a similar problem.
> 
> Found using coverity.
> 
> v2: initialize netlist to NULL

I'm fine with the patch as is (so "ACK"), but I'm wondering about
something.

> @@ -642,7 +642,6 @@ init_route_list (struct route_list *rl,
>       else
>         {
>              struct addrinfo* curele;
> -            gc_addspecial(netlist, &gc_freeaddrinfo_callback, &gc);
>              for (curele      = netlist; curele; curele = curele->ai_next)
>             {
>                  struct route_ipv4 *new;
> @@ -653,6 +652,8 @@ init_route_list (struct route_list *rl,
>                  rl->routes = new;
>             }
>         }
> +     if (netlist)
> +       gc_addspecial(netlist, &gc_freeaddrinfo_callback, &gc);
>        }
>    }

I can see why we do this gc_addspecial() dance in socket.c (because the
elements returned by getaddrinfo() are being pointed at, not copied).

This usage here seems unneeded, as the code will walk netlist, copy
out the IPv4 addresses into a separate (gc-allocated) struct route_ipv4,
and never use parts of netlist again - so freeaddrinfo() *should* work,
and make the code slightly less complex.

Arne, this came from you in commit e719a053534 (and I ACKed it) - could
you revisit this particular use and check whether it's truly needed?

thanks,

gert


-- 
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             g...@greenie.muc.de
fax: +49-89-35655025                        g...@net.informatik.tu-muenchen.de

Attachment: signature.asc
Description: PGP signature

Reply via email to