Re: [PATCH v2] resolv: add IPv6 support to inet_net_pton()

[Carlos O'Donell <carlos@redhat.com>]

On 4/16/24 12:59, Job Snijders wrote:
> Dear all,
> 
> The below patch is a new revision to add IPv6 support to
> inet_net_pton(). The patch is based on the OpenBSD implementation.
> 
> Kind regards,
> 
> Job
> 
> Signed-off: Job Snijders <job@fastly.com>

In my context as a project steward, and because DJ asked me to look at this patch
with regards to the license changes, I'm going to ask some clarifying questions to
make sure we respect your wishes as the contributor of the patch.

For the review I will note that:

(a) You are using DCO...

> ---
>  resolv/inet_net_pton.c | 69 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 65 insertions(+), 4 deletions(-)
> 
> diff --git a/resolv/inet_net_pton.c b/resolv/inet_net_pton.c
> index 63a47b7394..93753cb37f 100644
> --- a/resolv/inet_net_pton.c
> +++ b/resolv/inet_net_pton.c
> @@ -1,4 +1,6 @@
>  /*
> + * Copyright (c) 2024 Job Snijders <job@fastly.com>
> + * Copyright (c) 2012 by Gilles Chehade <gilles@openbsd.org>

... and (b) You add new copyright notices for specific individuals to the ISC
license portion.

Now the questions:

(1) May you please confirm that you are contributing your own changes under
    the ISC License?

    - Gilles Chehade <gilles@openbsd.org> changes were already contributed
      under that license to OpenBSD.

    - The other acceptable license in this case, and it's up to you, would
      be LGPLv2.1+. The choice here is a balance between what glibc
      will accept e.g. compatible free license, and what you want to
      contribute under. If you are considering keeping this code in sync
      with OpenBSD, then it would be simpler to stay with the ISC License
      that we already have here.

(2) Would you be amenable to using "Copyright The GNU Toolchain Authors."
    as your notice instead?

    - You are absolutely entitled to a copyright notice if you request
      it, but glibc project maintenance is simplified by a using a general
      notice.
      Please see: https://www.linuxfoundation.org/blog/blog/copyright-notices-in-open-source-software-projects

    - The notice from Gilles Chehade must be preserved because it comes
      from the copy of the OpenBSD implementation.

Thank you for contributing to glibc! :-)

I know Florian had some further down-thread suggestions, but the license
and copyright issues still apply.

>   * Copyright (c) 1996,1999 by Internet Software Consortium.
>   *
>   * Permission to use, copy, modify, and distribute this software for any
> @@ -19,6 +21,7 @@
>  #include <sys/socket.h>
>  #include <netinet/in.h>
>  #include <arpa/inet.h>
> +#include <resolv/resolv-internal.h>
>  
>  #include <assert.h>
>  #include <ctype.h>
> @@ -35,13 +38,16 @@
>  
>  static int	inet_net_pton_ipv4 (const char *src, u_char *dst,
>  				    size_t size) __THROW;
> +static int	inet_net_pton_ipv6 (const char *src, u_char *dst,
> +				    size_t size) __THROW;
>  
>  /*
> - * static int
> + * int
>   * inet_net_pton(af, src, dst, size)
> - *	convert network number from presentation to network format.
> - *	accepts hex octets, hex strings, decimal octets, and /CIDR.
> - *	"size" is in bytes and describes "dst".
> + *	Convert network number from presentation format to network format.
> + *	If "af" is set to AF_INET, accept various formats like hex octets,
> + *	hex strings, or decimal octets. If "af" is set to AF_INET6, accept
> + *	IPv6 addresses. "size" is in bytes and describes "dst".
>   * return:
>   *	number of bits, either imputed classfully or specified with /CIDR,
>   *	or -1 if some failure occurred (check errno).  ENOENT means it was
> @@ -55,6 +61,8 @@ inet_net_pton (int af, const char *src, void *dst, size_t size)
>  	switch (af) {
>  	case AF_INET:
>  		return (inet_net_pton_ipv4(src, dst, size));
> +	case AF_INET6:
> +		return (inet_net_pton_ipv6(src, dst, size));
>  	default:
>  		__set_errno (EAFNOSUPPORT);
>  		return (-1);
> @@ -196,3 +204,56 @@ inet_net_pton_ipv4 (const char *src, u_char *dst, size_t size)
>  	__set_errno (EMSGSIZE);
>  	return (-1);
>  }
> +
> +
> +/*
> + * Convert an IPv6 prefix from presentation format to network format.
> + * Return the number of bits specified, or -1 as error (check errno).
> + */
> +static int
> +inet_net_pton_ipv6 (const char *src, u_char *dst, size_t size)
> +{
> +	struct in6_addr	 in6;
> +	int		 bits;
> +	long		 lbits;
> +	size_t		 bytes;
> +	char		*ep, *sep;
> +
> +	sep = strchr(src, '/');
> +
> +	if (__inet_pton_length(AF_INET6, src, sep ? sep - src : strlen(src),
> +	    &in6) != 1) {
> +		__set_errno (ENOENT);
> +		return (-1);
> +	}
> +
> +	if (sep == NULL) {
> +		bits = 128;
> +		goto out;
> +	}
> +
> +	if (sep[0] == '\0' || !isascii(sep[0]) || !isdigit(sep[0])) {
> +		__set_errno (ENOENT);
> +		return (-1);
> +	}
> +
> +	lbits = strtol(sep, &ep, 10);
> +	if (ep == sep || *ep != '\0') {
> +		__set_errno (ENOENT);
> +		return (-1);
> +	}
> +	if (lbits < 0 || lbits > 128) {
> +		__set_errno (EMSGSIZE);
> +		return (-1);
> +	}
> +	bits = lbits;
> +
> + out:
> +	bytes = (bits + 7) / 8;
> +	if (bytes > size) {
> +		__set_errno (EMSGSIZE);
> +		return (-1);
> +	}
> +	memcpy(dst, &in6.s6_addr, bytes);
> +	return (bits);
> +}
> 

-- 
Cheers,
Carlos.