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.
--
:wq Claudio
>
> Index: tcpdrop.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/tcpdrop/tcpdrop.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 tcpdrop.c
> --- tcpdrop.c 29 Jun 2014 00:58:45 -0000 1.14
> +++ tcpdrop.c 23 Dec 2014 03:58:56 -0000
> @@ -47,7 +47,7 @@ main(int argc, char **argv)
> char lhbuf[NI_MAXHOST], lsbuf[NI_MAXSERV];
> char *laddr1, *addr1, *port1, *faddr2, *addr2, *port2;
> struct tcp_ident_mapping tir;
> - int gaierr, rval = 0;
> + int gaierr = 0, rval = 0;
>
> memset(&hints, 0, sizeof(hints));
> hints.ai_family = AF_UNSPEC;
> @@ -125,11 +125,11 @@ fail:
> if ((gaierr = getnameinfo(aif->ai_addr, aif->ai_addrlen,
> fhbuf, sizeof(fhbuf), fsbuf, sizeof(fsbuf),
> NI_NUMERICHOST | NI_NUMERICSERV)) != 0)
> - errx(1, "getnameinfo: %s",
> gai_strerror(gaierr));
> + goto done;
> if ((gaierr = getnameinfo(ail->ai_addr, ail->ai_addrlen,
> lhbuf, sizeof(lhbuf), lsbuf, sizeof(lsbuf),
> NI_NUMERICHOST | NI_NUMERICSERV)) != 0)
> - errx(1, "getnameinfo: %s",
> gai_strerror(gaierr));
> + goto done;
>
> if (sysctl(mib, sizeof (mib) / sizeof (int), NULL,
> NULL, &tir, sizeof(tir)) == -1) {
> @@ -145,7 +145,10 @@ fail:
> }
> }
> }
> +done:
> freeaddrinfo(laddr);
> freeaddrinfo(faddr);
> + if (gaierr)
> + errx(1, "getnameinfo: %s", gai_strerror(gaierr));
> exit(rval);
> }
>
