On 2/10/26 18:10, Peter Krempa wrote:
> On Fri, Feb 06, 2026 at 14:52:35 +0100, Michal Privoznik via Devel wrote:
>> From: Michal Privoznik <[email protected]>
>>
>> The aim of this helper is to convert subnet mask to prefix. For
>> instance for input "255.0.0.0" to return 0. Additionally, if the
>> input string is already a prefix (with optional leading slash
>> character) just return that number parsed.
>>
>> Signed-off-by: Michal Privoznik <[email protected]>
>> ---
>> src/libvirt_private.syms | 1 +
>> src/util/virsocketaddr.c | 51 ++++++++++++++++++++++++++++++++++++++++
>> src/util/virsocketaddr.h | 2 ++
>> tests/sockettest.c | 29 +++++++++++++++++++++++
>> 4 files changed, 83 insertions(+)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index d81b30f0b6..035eccf70a 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -3461,6 +3461,7 @@ virSocketAddrSetIPv4AddrNetOrder;
>> virSocketAddrSetIPv6Addr;
>> virSocketAddrSetIPv6AddrNetOrder;
>> virSocketAddrSetPort;
>> +virSocketAddrSubnetToPrefix;
>>
>>
>> # util/virstoragefile.h
>> diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c
>> index 1f203fb50d..89d41d7656 100644
>> --- a/src/util/virsocketaddr.c
>> +++ b/src/util/virsocketaddr.c
>> @@ -21,6 +21,7 @@
>> #include "virsocketaddr.h"
>> #include "virerror.h"
>> #include "virbuffer.h"
>> +#include "virstring.h"
>>
>> #define VIR_FROM_THIS VIR_FROM_NONE
>>
>> @@ -1263,6 +1264,56 @@ virSocketAddrNumericFamily(const char *address)
>> return family;
>> }
>>
>> +/**
>> + * virSocketAddrSubnetToPrefix:
>> + * @subnet: address to convert
>> + *
>> + * Converts subnet mask to prefix. If @subnet is of an IPv4
>> + * format (NNN.NNN.NNN.NNN) then corresponding prefix length is
>> + * returned (i.e. number of leading bits.). If @subnet is just a
>> + * number (optionally prefixed with '/') then the number is
>> + * parsed and returned.
>
> This doesn't say any caveats if e.g. the IPv4 string you pass in isn't a
> subnet mask at all ...
>
>> + * Returns: prefix corresponding to @subnet,
>> + * -1 otherwise.
>> + */
>> +int
>> +virSocketAddrSubnetToPrefix(const char *subnet)
>> +{
>> + struct addrinfo *ai = NULL;
>> + unsigned int prefix = 0;
>> + struct sockaddr_in in;
>> + int ret = -1;
>> +
>> + if (*subnet == '/') {
>> + /* /NN format */
>> + if (virStrToLong_ui(subnet + 1, NULL, 10, &prefix) < 0)
>> + return -1;
>> + return prefix;
>> + }
>> +
>> + if (virStrToLong_ui(subnet, NULL, 10, &prefix) >= 0) {
>> + /* plain NN format */
>> + return prefix;
>> + }
>> +
>> + if (virSocketAddrParseInternal(&ai, subnet, AF_INET, AI_NUMERICHOST,
>> false) < 0)
>> + return -1;
>> +
>> + if (ai->ai_family != AF_INET) {
>> + /* huh? */
>> + goto cleanup;
>> + }
>> +
>> + memcpy(&in, ai->ai_addr, sizeof(in));
>> + prefix = __builtin_popcount(in.sin_addr.s_addr);
>
> ... so if you don't pass a mask it gives not really documented answers.
>
> With the impl above the following predicates of the test added later
> are also true:
>
> DO_TEST_SUBNET_TO_PREFIX("192.168.0.1", 6);
> DO_TEST_SUBNET_TO_PREFIX("129.0.0.1", 3);
>
>
> Depending on the expectations either document the expectation of what
> happens argument isn't a subnet or add some checks
>
> E.g. you could use __builtin_ffs to figure out the number of leading
> bits and then compare with __builtin_popcount to see if other bits
> aren't set.
Would this work on big endian machines? I mean, even on my little endian
machine 255.255.255.254 is stored as 0xfeffffff. So I guess I'll just
update documentation. How about:
* parsed and returned. There is a corner case: if @subnet is
* valid IPv4 address but not valid subnet mask then a positive
* value is returned, but obviously it is not valid prefix.
>
>
>> diff --git a/tests/sockettest.c b/tests/sockettest.c
>> index 5cb8a9fb72..df62dc6f3b 100644
>> --- a/tests/sockettest.c
>> +++ b/tests/sockettest.c
>> @@ -257,6 +257,21 @@ testIsLocalhostHelper(const void *opaque)
>> return 0;
>> }
>>
>> +struct testSubnetToPrefixData {
>> + const char *addr;
>> + int prefix;
>> +};
>> +
>> +static int
>> +testSubnetToPrefixHelper(const void *opaque)
>> +{
>> + const struct testSubnetToPrefixData *data = opaque;
>> +
>> + if (virSocketAddrSubnetToPrefix(data->addr) != data->prefix)
>> + return -1;
>
> Consider adding:
>
> diff --git a/tests/sockettest.c b/tests/sockettest.c
> index df62dc6f3b..37f654880a 100644
> --- a/tests/sockettest.c
> +++ b/tests/sockettest.c
> @@ -266,9 +266,13 @@ static int
> testSubnetToPrefixHelper(const void *opaque)
> {
> const struct testSubnetToPrefixData *data = opaque;
> + int val = virSocketAddrSubnetToPrefix(data->addr);
>
> - if (virSocketAddrSubnetToPrefix(data->addr) != data->prefix)
> + if (val != data->prefix) {
> + fprintf(stderr, "actual: '%d', expected: '%d'", val, data->prefix);
> return -1;
> + }
> +
> return 0;
> }
>
> for debuggability
>
Squashed in locally.
Michal
