[libvirt] [PATCHv2 1/2] util: set missing data length in virSocketAddrPrefixToNetmask()

[Laine Stump]

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 at 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 at 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)