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@xxxxxxxxx>
---
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@xxxxxxxxxx>
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)
