On Mon, Dec 20, 2010 at 09:03, Laine Stump <laine@xxxxxxxxx> wrote:
> diff --git a/src/util/network.c b/src/util/network.c
> index 1abe78b..e4791b9 100644
> --- a/src/util/network.c
> +++ b/src/util/network.c
> @@ -288,6 +288,73 @@ int virSocketAddrIsNetmask(virSocketAddrPtr netmask) {
> }
>
> /**
> + * virSocketAddrMask:
> + * @addr: address that needs to be masked
> + * @netmask: the netmask address
> + *
> + * Mask off the host bits of @addr according to @netmask, turning it
> + * into a network address.
> + *
> + * Returns 0 in case of success, or -1 on error.
> + */
> +int
> +virSocketAddrMask(virSocketAddrPtr addr, const virSocketAddrPtr netmask)
> +{
> + if (addr->data.stor.ss_family != netmask->data.stor.ss_family)
> + return -1;
> +
> + if (addr->data.stor.ss_family == AF_INET) {
> + addr->data.inet4.sin_addr.s_addr
> + &= netmask->data.inet4.sin_addr.s_addr;
> + return 0;
> + }
> + if (addr->data.stor.ss_family == AF_INET6) {
> + int ii;
> + for (ii = 0; ii < 4; ii++)
> + addr->data.inet6.sin6_addr.s6_addr32[ii]
> + &= netmask->data.inet6.sin6_addr.s6_addr32[ii];
> + return 0;
> + }
> + return -1;
> +}
> +
> +/**
> + * virSocketAddrMaskByPrefix:
> + * @addr: address that needs to be masked
> + * @prefix: prefix (# of 1 bits) of netmask to apply
> + *
> + * Mask off the host bits of @addr according to @prefix, turning it
> + * into a network address.
> + *
> + * Returns 0 in case of success, or -1 on error.
> + */
> +int
> +virSocketAddrMaskByPrefix(virSocketAddrPtr addr, int prefix)
> +{
> + virSocketAddr netmask;
> +
> + if (virSocketAddrPrefixToNetmask(prefix, &netmask,
> + addr->data.stor.ss_family) < 0)
> + return -1;
why not replace following:
> + if (addr->data.stor.ss_family == AF_INET) {
> +
> + addr->data.inet4.sin_addr.s_addr
> + &= netmask.data.inet4.sin_addr.s_addr;
> + return 0;
> + }
> + if (addr->data.stor.ss_family == AF_INET6) {
> + int ii;
> +
> + for (ii = 0; ii < 4; ii++)
> + addr->data.inet6.sin6_addr.s6_addr32[ii]
> + &= netmask.data.inet6.sin6_addr.s6_addr32[ii];
> + return 0;
> + }
> + return -1;
> +}
with simple:
return virSocketAddrMask(addr, netmask);
With cost of checking ss_family for addr and netmask we achieve clearer code.
> +/**
> + * virSocketPrefixToNetmask:
> + * @prefix: number of 1 bits to put in the netmask
> + * @netmask: address to fill in with the desired netmask
> + * @family: family of the address (AF_INET or AF_INET6 only)
> + *
> + * given @prefix and @family, fill in @netmask with a netmask
> + * (eg 255.255.255.0).
> + *
> + * Returns 0 on success or -1 on error.
> + */
> +
> +int virSocketAddrPrefixToNetmask(int prefix,
> + virSocketAddrPtr netmask,
> + int family)
> +{
> + int result = -1;
> +
> + netmask->data.stor.ss_family = AF_UNSPEC; /* assume failure */
You don't check if (prefix < 0) for AF_INET6. So my suggestion is to
check it here:
if (prefix < 0)
goto error;
> + if (family == AF_INET) {
> + int ip = 0;
> +
and remove this check from here:
> + if (prefix < 0 || prefix > 32)
> + goto error;
> +
btw: does prefix really needs to be signed int?
--
Pawel
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list