Re: [PATCH net-next v2 3/5] netconsole: add support for strings with new line in netpoll_parse_ip_addr

[Simon Horman <horms@kernel.org>]

On Mon, Jul 21, 2025 at 06:02:03AM -0700, Breno Leitao wrote:
> The current IP address parsing logic fails when the input string
> contains a trailing newline character. This can occur when IP
> addresses are provided through configfs, which contains newlines in
> a const buffer.
> 
> Teach netpoll_parse_ip_addr() how to ignore newlines at the end of the
> IPs. Also, simplify the code by:
> 
>  * No need to check for separators. Try to parse ipv4, if it fails try
>    ipv6 similarly to ceph_pton()
>  * If ipv6 is not supported, don't call in6_pton() at all.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

My suggestion below not withstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

> ---
>  drivers/net/netconsole.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 8d1b93264e0fd..f2c2b8852c603 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -303,20 +303,21 @@ static void netconsole_print_banner(struct netpoll *np)
>  static int netpoll_parse_ip_addr(const char *str, union inet_addr *addr)
>  {
>  	const char *end;
> +	int len;
>  
> -	if (!strchr(str, ':') &&
> -	    in4_pton(str, -1, (void *)addr, -1, &end) > 0) {
> -		if (!*end)
> -			return 0;
> -	}
> -	if (in6_pton(str, -1, addr->in6.s6_addr, -1, &end) > 0) {
> -#if IS_ENABLED(CONFIG_IPV6)
> -		if (!*end)
> -			return 1;
> -#else
> +	len = strlen(str);
> +	if (!len)
>  		return -1;
> +
> +	if (str[len - 1] == '\n')
> +		len -= 1;
> +
> +	if (in4_pton(str, len, (void *)addr, -1, &end) > 0)
> +		return 0;
> +#if IS_ENABLED(CONFIG_IPV6)
> +	if (in6_pton(str, len, addr->in6.s6_addr, -1, &end) > 0)
> +		return 1;
>  #endif

I don't think it needs to block progress.
But FWIIW, I think it would be nice to increase
build coverage and express this as:

	if (IS_ENABLED(CONFIG_IPV6) &&
	    in6_pton(str, len, addr->in6.s6_addr, -1, &end) > 0)
		return 1;