On 2/20/19 4:10 PM, John Ferlan wrote:
On 2/18/19 6:21 PM, Laine Stump wrote:
This fixes a bug that has been present since the original version of the function was pushed in commit 1ab80f3 on Nov. 26 2010 (by me). The virSocketAddr::len was not being set.
Apparently until now we were always calling virSocketAddrPrefixToNetmask() with a virSocketAddr object that was already (coincidentally) initialized for the proper address family, but the bug became apparent when trying to use it to fill in an otherwise uninitialized object.
Signed-off-by: Laine Stump <laine@laine.org> --- src/util/virsocketaddr.c | 2 ++ 1 file changed, 2 insertions(+)
I'm OK with this change as is - just curious on the below query...
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 4bc14bbd15..ccfaeabe13 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -1032,6 +1032,7 @@ virSocketAddrPrefixToNetmask(unsigned int prefix, ip = prefix ? ~((1 << (32 - prefix)) - 1) : 0; netmask->data.inet4.sin_addr.s_addr = htonl(ip); netmask->data.stor.ss_family = AF_INET; + netmask->len = sizeof(struct sockaddr_in);
Hmmm.. how similar to virSocketAddrSetIPv4AddrNetOrder overall? I was looking for "data\.inet.*=" via cscope and found that...
Interesting. These 3 lines *are* identical in function to calling virSocketAddrSetIPv4Addr() (the one that does an htonl() on the address).
result = 0;
} else if (family == AF_INET6) { @@ -1055,6 +1056,7 @@ virSocketAddrPrefixToNetmask(unsigned int prefix, netmask->data.inet6.sin6_addr.s6_addr[i++] = 0; } netmask->data.stor.ss_family = AF_INET6; + netmask->len = sizeof(struct sockaddr_in6);
My brain hurts thinking if similar to virSocketAddrSetIPv6AddrNetOrder
Yeah. I'm not in the mood to figure out what's the right byte order for the input to what happens in virSocketAddrSetIPv6Addr() and virSocketAddrSetIPv6AddrNetOrder() is identical to what's currently done in the IPv6 clause of virSocketAddrPrefixToNetmask(), so I think I'd rather leave all of it for clarity's sake (and also to avoid the potential of having my bug fix create yet another regression :-P)