On Tue, Dec 30, 2014 at 07:44:51AM +0100, Claudio Jeker wrote:
> On Tue, Dec 30, 2014 at 12:00:38AM -0500, Lawrence Teo wrote:
> > tcpdrop makes two getaddrinfo() calls:
> >
> > if ((gaierr = getaddrinfo(laddr1, port1, &hints, &laddr)) != 0)
> > errx(1, "%s port %s: %s", addr1, port1,
> > gai_strerror(gaierr));
> >
> > if ((gaierr = getaddrinfo(faddr2, port2, &hints, &faddr)) != 0) {
> > freeaddrinfo(laddr);
> > errx(1, "%s port %s: %s", addr2, port2,
> > gai_strerror(gaierr));
> > }
> >
> > However, their corresponding freeaddrinfo() calls will not be called if
> > either of the two getnameinfo() calls in the subsequent for loop fails.
> >
> > This diff ensures that both freeaddrinfo() calls are called in that
> > error path.
> >
> > ok?
> >
>
> I see no need for this. errx() does not return and it frees all the memory
> including the one allocated by getaddrinfo(). Why make this more
> complicated when the program just calls exit as next?
> I would even remove the freeaddrinfo(laddr); in the second case.
>
Claudio, thank you for the feedback; it makes the diff much simpler. :)
Index: tcpdrop.c
===================================================================
RCS file: /cvs/src/usr.sbin/tcpdrop/tcpdrop.c,v
retrieving revision 1.14
diff -u -p -u -p -r1.14 tcpdrop.c
--- tcpdrop.c 29 Jun 2014 00:58:45 -0000 1.14
+++ tcpdrop.c 31 Dec 2014 03:03:50 -0000
@@ -106,11 +106,9 @@ fail:
errx(1, "%s port %s: %s", addr1, port1,
gai_strerror(gaierr));
- if ((gaierr = getaddrinfo(faddr2, port2, &hints, &faddr)) != 0) {
- freeaddrinfo(laddr);
+ if ((gaierr = getaddrinfo(faddr2, port2, &hints, &faddr)) != 0)
errx(1, "%s port %s: %s", addr2, port2,
gai_strerror(gaierr));
- }
rval = 1;
for (ail = laddr; ail; ail = ail->ai_next) {
