[libvirt] [PATCH 3/3] Improve virSocketAddrMask[ByPrefix] API

[Laine Stump]

On 12/31/2010 08:13 AM, Stefan Berger wrote:
> On 12/31/2010 06:33 AM, Laine Stump wrote:
>> The original version of these functions would modify the address sent
>> in, meaning that the caller would usually need to copy the address
>> first. This change makes the original a const, and puts the resulting
>> masked address into a new arg (which could point to the same
>> virSocketAddr as the original, if the caller really wants to modify
>> it).
>>
>> This also makes the API consistent with 
>> virSocketAddrBroadcast[ByPrefix].
>> ---
>>   src/util/iptables.c |    3 +--
>>   src/util/network.c  |   36 ++++++++++++++++++++++++++----------
>>   src/util/network.h  |   10 ++++++----
>>   3 files changed, 33 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/util/iptables.c b/src/util/iptables.c
>> index 647e7ae..6770fe0 100644
>> --- a/src/util/iptables.c
>> +++ b/src/util/iptables.c
>> @@ -298,8 +298,7 @@ static char *iptablesFormatNetwork(virSocketAddr 
>> *netaddr,
>>           return NULL;
>>       }
>>
>> -    network = *netaddr;
>> -    if (virSocketAddrMaskByPrefix(&network, prefix)<  0) {
>> +    if (virSocketAddrMaskByPrefix(netaddr, prefix,&network)<  0) {
>>           iptablesError(VIR_ERR_INTERNAL_ERROR, "%s",
>>                         _("Failure to mask address"));
>>           return NULL;
>> diff --git a/src/util/network.c b/src/util/network.c
>> index f58986e..a7e7423 100644
>> --- a/src/util/network.c
>> +++ b/src/util/network.c
>> @@ -298,23 +298,35 @@ int virSocketAddrIsNetmask(virSocketAddrPtr 
>> netmask) {
>>    * Returns 0 in case of success, or -1 on error.
>>    */
>>   int
>> -virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr 
>> netmask)
>> +virSocketAddrMask(const virSocketAddrPtr addr,
>> +                  const virSocketAddrPtr netmask,
>> +                  virSocketAddrPtr       network)
>>   {
>> -    if (addr->data.stor.ss_family != netmask->data.stor.ss_family)
>> +    if (addr->data.stor.ss_family != netmask->data.stor.ss_family) {
>> +        network->data.stor.ss_family = AF_UNSPEC;
>>           return -1;
>> +    }
>>
>>       if (addr->data.stor.ss_family == AF_INET) {
>> -        addr->data.inet4.sin_addr.s_addr
>> -&= netmask->data.inet4.sin_addr.s_addr;
>> +        network->data.inet4.sin_addr.s_addr
>> +            = (addr->data.inet4.sin_addr.s_addr
>> +&  netmask->data.inet4.sin_addr.s_addr);
>> +        network->data.stor.ss_family = AF_INET;
>> +        network->len = addr->len;
>>           return 0;
>>       }
>>       if (addr->data.stor.ss_family == AF_INET6) {
>>           int ii;
>> -        for (ii = 0; ii<  16; ii++)
>> -            addr->data.inet6.sin6_addr.s6_addr[ii]
>> -&= netmask->data.inet6.sin6_addr.s6_addr[ii];
>> +        for (ii = 0; ii<  16; ii++) {
>> +            network->data.inet6.sin6_addr.s6_addr[ii]
>> +                = (addr->data.inet6.sin6_addr.s6_addr[ii]
>> +&  netmask->data.inet6.sin6_addr.s6_addr[ii]);
>> +        }
>> +        network->data.stor.ss_family = AF_INET6;
>> +        network->len = addr->len;
>>           return 0;
>>       }
>> +    network->data.stor.ss_family = AF_UNSPEC;
>>       return -1;
>>   }
>>
>> @@ -329,15 +341,19 @@ virSocketAddrMask(virSocketAddrPtr addr, const 
>> virSocketAddrPtr netmask)
>>    * Returns 0 in case of success, or -1 on error.
>>    */
>>   int
>> -virSocketAddrMaskByPrefix(virSocketAddrPtr addr, unsigned int prefix)
>> +virSocketAddrMaskByPrefix(const virSocketAddrPtr addr,
>> +                          unsigned int           prefix,
>> +                          virSocketAddrPtr       network)
>>   {
>>       virSocketAddr netmask;
>>
>>       if (virSocketAddrPrefixToNetmask(prefix,&netmask,
>> -                                     addr->data.stor.ss_family)<  0)
>> +                                     addr->data.stor.ss_family)<  0) {
>> +        network->data.stor.ss_family = AF_UNSPEC;
>>           return -1;
>> +    }
>>
>> -    return virSocketAddrMask(addr,&netmask);
>> +    return virSocketAddrMask(addr,&netmask, network);
>>   }
>>
>>   /**
>> diff --git a/src/util/network.h b/src/util/network.h
>> index bcbc607..0b43bf6 100644
>> --- a/src/util/network.h
>> +++ b/src/util/network.h
>> @@ -73,10 +73,12 @@ int virSocketAddrIsNetmask(virSocketAddrPtr 
>> netmask);
>>   int virSocketCheckNetmask (virSocketAddrPtr addr1,
>>                              virSocketAddrPtr addr2,
>>                              virSocketAddrPtr netmask);
>> -int virSocketAddrMask     (virSocketAddrPtr addr,
>> -                           const virSocketAddrPtr netmask);
>> -int virSocketAddrMaskByPrefix(virSocketAddrPtr addr,
>> -                              unsigned int prefix);
>> +int virSocketAddrMask     (const virSocketAddrPtr addr,
>> +                           const virSocketAddrPtr netmask,
>> +                           virSocketAddrPtr       network);
>> +int virSocketAddrMaskByPrefix(const virSocketAddrPtr addr,
>> +                              unsigned int           prefix,
>> +                              virSocketAddrPtr       network);
>>   int virSocketAddrBroadcast(const virSocketAddrPtr addr,
>>                              const virSocketAddrPtr netmask,
>>                              virSocketAddrPtr       broadcast);
> Looks good to me. ACK.
>     Stefan

Since this is just a change to new code since the last release, and 
testing shows no regressions, I've pushed it so it doesn't get lost.