[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@xxxxxxxxx> wrote:
>
>> Hi Marko,
>>
>> On Thu, 17 Sep 2020 at 07:34, Marko Hrastovec <marko.hrastovec@xxxxxxxxx>
>> wrote:
>>>
>>> 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.
>>>
>>> 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/
