[PATCH] conf: Don't pass leading ~ to parse_port_range() on exclusions

[PATCH] conf: Don't pass leading ~ to parse_port_range() on exclusions

[Stefano Brivio]

Commit 84fec4e998b6 ("Clean up parsing of port ranges") drops the
strspn() call before the parsing of excluded port ranges, because now
we're checking against any stray characters at every step.

However, that also has the effect of passing ~ as first character to
the new parse_port_range(), which makes no sense: we already checked
that ~ is the first character before the call, so skip it.

Alona reported this output:
  Invalid port specifier ~15000,~15001,~15006,~15008,~15020,~15021,~15090

while the whole specifier is indeed valid.

Reported-by: Alona Paz <alkaplan@redhat.com>
Fixes: 84fec4e998b6 ("Clean up parsing of port ranges")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
I'm not introducing new tests with this yet, this is another instance
where I would need a test case restarting pasta a bunch of times with
several different options instead of a fixed setup. I'll reopen the
discussion around:
  https://archives.passt.top/passt-dev/Yz%2FlXVVDgMEWweTj@yekko/#t

Also, I'm posting this for review, but again, let me push this out
quickly, as configuration of excluded ports is completely broken at
the moment.

 conf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/conf.c b/conf.c
index ed93a60..598c711 100644
--- a/conf.c
+++ b/conf.c
@@ -262,6 +262,7 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 			exclude_only = false;
 			continue;
 		}
+		p++;
 
 		if (parse_port_range(p, &p, &xrange))
 			goto bad;
-- 
@@ -262,6 +262,7 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
 			exclude_only = false;
 			continue;
 		}
+		p++;
 
 		if (parse_port_range(p, &p, &xrange))
 			goto bad;
-- 
2.35.1

Re: [PATCH] conf: Don't pass leading ~ to parse_port_range() on exclusions

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1954 bytes --]

On Mon, Oct 24, 2022 at 03:04:59PM +0200, Stefano Brivio wrote:
> Commit 84fec4e998b6 ("Clean up parsing of port ranges") drops the
> strspn() call before the parsing of excluded port ranges, because now
> we're checking against any stray characters at every step.
> 
> However, that also has the effect of passing ~ as first character to
> the new parse_port_range(), which makes no sense: we already checked
> that ~ is the first character before the call, so skip it.
> 
> Alona reported this output:
>   Invalid port specifier ~15000,~15001,~15006,~15008,~15020,~15021,~15090
> 
> while the whole specifier is indeed valid.
> 
> Reported-by: Alona Paz <alkaplan@redhat.com>
> Fixes: 84fec4e998b6 ("Clean up parsing of port ranges")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Oops, sorry about that.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> I'm not introducing new tests with this yet, this is another instance
> where I would need a test case restarting pasta a bunch of times with
> several different options instead of a fixed setup. I'll reopen the
> discussion around:
>   https://archives.passt.top/passt-dev/Yz%2FlXVVDgMEWweTj@yekko/#t
> 
> Also, I'm posting this for review, but again, let me push this out
> quickly, as configuration of excluded ports is completely broken at
> the moment.
> 
>  conf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/conf.c b/conf.c
> index ed93a60..598c711 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -262,6 +262,7 @@ static int conf_ports(const struct ctx *c, char optname, const char *optarg,
>  			exclude_only = false;
>  			continue;
>  		}
> +		p++;
>  
>  		if (parse_port_range(p, &p, &xrange))
>  			goto bad;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]