Re: [patch] freeaddrinfo.3: memory leaks in freeaddrinfo examples

[CC += beej, to alert the author about the memory leaks 
in the network programming guide]

Hello Marko,

On Thu, Sep 17, 2020 at 7:42 AM Michael Kerrisk (man-pages) <
mtk.manpages@gmail.com> wrote:

quoted

Hi Marko,

On Thu, 17 Sep 2020 at 07:34, Marko Hrastovec [off-list ref]
wrote:

quoted

Hi,

examples in freeaddrinfo.3 have a memory leak, which is replicated in

many real world programs copying an example from manual pages. The two
examples should have different order of lines, which is done in the
following patch.

quoted

diff --git a/man3/getaddrinfo.3 b/man3/getaddrinfo.3
index c9a4b3e43..4d383bea0 100644
--- a/man3/getaddrinfo.3
+++ b/man3/getaddrinfo.3

@@ -711,13 +711,13 @@ main(int argc, char *argv[])
         close(sfd);
     }

+    freeaddrinfo(result);           /* No longer needed */
+
     if (rp == NULL) {               /* No address succeeded */
         fprintf(stderr, "Could not bind\en");
         exit(EXIT_FAILURE);
     }

-    freeaddrinfo(result);           /* No longer needed */
-
     /* Read datagrams and echo them back to sender */

     for (;;) {

@@ -804,13 +804,13 @@ main(int argc, char *argv[])
         close(sfd);
     }

+    freeaddrinfo(result);           /* No longer needed */
+
     if (rp == NULL) {               /* No address succeeded */
         fprintf(stderr, "Could not connect\en");
         exit(EXIT_FAILURE);
     }

-    freeaddrinfo(result);           /* No longer needed */
-
     /* Send remaining command\-line arguments as separate
        datagrams, and read responses from server */

When you say "memory leak", do you mean that something like valgrind
complains? I mean, strictly speaking, there is no memory leak that I
can see that is fixed by that patch, since the if-branches that the
freeaddrinfo() calls are shifted above terminates the process in each
case.

you are right about terminating the process. However, people copy that
example and put the code in function changing "exit" to "return". There are
a bunch of examples like that here https://beej.us/guide/bgnet/html/#poll,
for instance.

Oh -- I see what you mean.

That error bothered me when reading the network programming
guide https://beej.us/guide/bgnet/html/. Than I looked for information
elsewhere:
-
https://stackoverflow.com/questions/6712740/valgrind-reporting-that-getaddrinfo-is-leaking-memory
-
https://stackoverflow.com/questions/15690303/server-client-sockets-freeaddrinfo3-placement
And finally, I checked manual pages and saw where these errors come from.

When you change that to a function and return without doing freeaddrinfo,
that is a memory leak. I believe an example should show good programming
practices. Relying on exiting and clearing the memory in that case is not
such a case. In my opinion, these examples lead people to make mistakes in
their programs.

Yes, I can buy that argument. I've applied your patch.

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/