[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Sat, 23 Nov 2019 15:46:44 +0000
From: gilles@...lp.org
To: musl@...ts.openwall.com
Subject: freeaddrinfo() comments and questions
Hello,
I'm not subscribed to the list, please do keep me cc-ed.
This is just a mail to express my opinion with regard to a somewhat recent commit,
feel free to disregard :-)
According to:
https://pubs.opengroup.org/onlinepubs/009695399/functions/freeaddrinfo.html
The freeaddrinfo() function shall free one or more addrinfo structures returned by getaddrinfo(), along with any additional storage associated with those structures. If the ai_next field of the structure is not null, the entire list of structures shall be freed. The freeaddrinfo() function shall support the freeing of arbitrary sublists of an addrinfo list originally returned by getaddrinfo().
In this commit:
https://git.musl-libc.org/cgit/musl/commit/src/network/freeaddrinfo.c?id=d1395c43c019aec6b855cf3c656bf47c8a719e7f
Rich Felker states:
the specification for freeaddrinfo allows it to be used to free
"arbitrary sublists" of the list returned by getaddrinfo. it's not
clearly stated how such sublists come into existence, but the
interpretation seems to be that the application can edit the ai_next
pointers to cut off a portion of the list and then free it.
I think that the reason why it's not clearly stated how such sublists come into existence...
is because of the mere definition of a sublist:
sublist. Noun. (plural sublists) A list that makes up part of a larger list.
Given that struct addrinfo is a linked list this doesn't give much room for interpretation,
a sublist of the struct addrinfo is just any point after the first node,
because that's the only way you can have a list that makes up part of a larger list.
This leads to my second point, the musl interpretation of the standard leads to an implementation which is not only at odds with what is found in pretty much every other libc, ranging from BSD to glibc, but also including Android, Solaris and others, but which also prevents the writing of portable code which crafts a struct addrinfo. I understand that you may not have a goal to do exactly what all the others do, but if pretty much every one has the same understanding, it should at least raise some concern if you're diverging in my opinion.
In these other implementations, it is possible to write a custom struct addrinfo allocator and use freeaddrinfo() on it, just like it is possible to use getaddrinfo() and use a custom release function on it. This is not a very common use-case, granted, but it is one nonetheless, and one that works and has worked in a portable way for a long time across a wide variety of systems.
With musl, the way struct addrinfo is handled not only prevents this but also makes the struct addrinfo kind of a weird structure because it is public, as it should, but relies on an underlying structure, struct aibuf, which is not something a user of struct addrinfo would see. Furthermore, the fact that freeaddrinfo() relies on pointer arithmetic makes it really clear that struct addrinfo isn't really a linked list but that it's an array of linked elements, and while this may make it much simpler for musl to handle thread-safety and memory releasing, this is really something that breaks user expectations big time if they assume struct addrinfo to be a list and to work as a list.
I personally have a work-around but I really think that this commit was not right and wished to contribute with my 2 cents if it can provide a different point of view. I don't think any implementation relying on contiguous memory and a single release can possibly be right as it means the struct addrinfo is not a list, can't have sublists, can't be self-crafted or self-released.
Cheers,
Gilles
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
