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

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