Closed GoogleCodeExporter closed 9 years ago
The code is rather confusing. Is the goal to get two list of addrinfo's, the v6
addresses, and the v4 addresses? If so, why can't you create two lists, one v4
and one v6, doing something like:
addrinfo *ipv4_list = NULL;
addrinfo *ipv6_list = NULL;
ai = fai;
while (ai != NULL) {
addrinfo *next = ai->ai_next;
if (ai->ai_family == AF_INET) {
ai->ai_next = ipv4_list;
ipv4_list = ai;
}
// add similar for AF_INET6
ai = next;
}
To get the sorted list of all v4 and v6 addresses, you just track the tail of
the v6 linked list, and set its ai_next pointer to the v4 list.
Original comment by AaronMat...@gmail.com
on 15 Apr 2013 at 3:46
Yes the goal is to get two lists of addrinfo's for v6 and v4. The v6 is then
linked to v4 as a fallback so ndt falls back to only v4 (A v6 socket can do
both v4 and v6).
In theory we can make these lists by setting ai_next and as long as every
element ends up in a list and the tails get set to NULL we cannot possibly have
a loop. But the library implementation of getaddrinfo returns a list of
individually malloc()'d nodes. If the first element is a v4 address then only
v4's will be linked after it and freeaddrinfo() would leak all the v6's. We
could make sure we call freeaddrinfo() on the head of the v6 (which will have
the v4's after it) but then freeaddrinfo() is being called with a different
memory address which would fail in a library that implemented the getaddrinfo()
list in a single contiguous piece of memory.
So I think we have to consider the list returned by getaddrinfo() read-only and
avoid touching ai_next. Which is why I've needed use struct ai_node's to point
to the actual addrinfo's.
Original comment by rsanger...@gmail.com
on 15 Apr 2013 at 6:26
I think a clearer solution would probably look something like:
break out most of the existing OpenSocket function into an internal one that
takes the family and whether to instaniate v4-to-v6 as options.
Then have OpenSocket do:
return = NULL;
if (require v4) {
return = __OpenSocket(addr, AF_INET, 0);
}
else if (require v6) {
return = __OpenSocket(addr, AF_INET6, 0);
}
else {
return = __OpenSocket(addr, AF_INET6, 1);
if (!return) {
return = __OpenSocket(addr, AF_INET, 0);
}
}
This disentangles the odd logic of v4/v6 choice from the actual instantiation
of socket, and would allow some comments explaining the choice in the
OpenSocket function.
Original comment by AaronMat...@gmail.com
on 19 Apr 2013 at 5:05
I committed a change similar to what I described above into trunk. Take a look
at src/network.c to see what I did. I added an "int family" to the OpenSocket
parameters (only caller was CreateListenSocket...). I then have
CreateListenSocket call it first for v6 (assuming AF_INET6 is around), and then
if that doesn't work (or it's v4 only, or v6 isn't available), it tries v4.
I've tested locally, but it could use some further testing. However, since
bwctl does almost the same thing, I'm pretty confident in it.
Original comment by AaronMat...@gmail.com
on 21 Apr 2013 at 7:28
Looks good to me. A lot less code this way which is nice.
I've tested this myself and this works as expected.
Original comment by rsanger...@gmail.com
on 24 Apr 2013 at 1:04
Original issue reported on code.google.com by
rsanger...@gmail.com
on 14 Apr 2013 at 3:27Attachments: