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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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...

>          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

>          result = 0;
>      }
>  
> 


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux