From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top
Subject: Re: [PATCH 01/28] Clean up parsing of port ranges
Date: Thu, 29 Sep 2022 11:04:17 +1000
Message-ID: <YzTvEc0WeO/rJjyt@yekko>
In-Reply-To: <20220928225731.124f76d4@elisabeth>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============0296245468072151377=="

--===============0296245468072151377==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Sep 28, 2022 at 10:57:31PM +0200, Stefano Brivio wrote:
> On Wed, 28 Sep 2022 14:33:12 +1000
> David Gibson <david(a)gibson.dropbear.id.au> wrote:
>=20
> > conf_ports() parses ranges of ports for the -t, -u, -T and -U options.
> > The code is quite difficult to the follow, to the point that clang-tidy
> > and cppcheck disagree on whether one of the pointers can be NULL at some
> > points.
> >=20
> > Rework the code with the use of two new helper functions:
> >   * parse_port_range() operates a bit like strtoul(), but can parse a who=
le
> >     port range specification (e.g. '80' or '1000-1015')
> >   * next_chunk() does the necessary wrapping around strchr() to advance to
> >     just after the next given delimiter, while cleanly handling if there
> >     are no more delimiters
> >=20
> > The new version is easier to follow,
>=20
> Indeed. Just one excess whitespace:
>=20
> > [...]
> > +		if (*p =3D=3D ':') { /* There's a range to map to as well */
> > +			if (parse_port_range(p + 1, &p, &mapped_range))
> >  				goto bad;
> > -
> > -			if (start_dst > end_dst)	/* 22-80:8080:8022 */
> > +			if ((mapped_range.last - mapped_range.first) !=3D
> > +			    (orig_range.last - orig_range.first))
> >  				goto bad;
> > +		} else {
> > +			mapped_range =3D orig_range;
> > +		}
> > =20
> > -			if (end_dst !=3D -1 &&
> > -			    end_dst - start_dst !=3D end_src - start_src)
> > -				goto bad;		/* 22-81:8022:8080 */
> > -
> > -			for (i =3D start_src; i <=3D end_src; i++) {
> > -				if (bitmap_isset(fwd->map, i))
> > -					goto overlap;
> > +		if ((*p !=3D '\0')  && (*p !=3D ',')) /* Garbage after the ranges */
>=20
> ...here. I can drop it myself.

Actually, I might as well respin it quickly.  That way I can add some
of the extra background information you've supplied to the commit
messages, which might be useful for posterity.

--=20
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

--===============0296245468072151377==
Content-Type: application/pgp-signature
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="signature.asc"
MIME-Version: 1.0

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUVCQ0FBZEZpRUVvVUx4V3U0L1dz
MGRCK1h0Z3lwWTRnRXdZU0lGQW1NMDd3c0FDZ2tRZ3lwWTRnRXcKWVNMdFd3LzhEZ1pWc0FrbkxU
T3ExVVFvcUVlQ09TTnk3bXJ6bjhjUkgza1M5SWN1L1AvS0xPaHdxRStHK0tUKwphNDg1eG1xakFo
dkJ4citOTGwxYjZXNWpkOTFtSEZGdWpTUjhXRnlkVWVZSUMyUWZlVkQ3eUpvMktnb2hJdVVOCkIv
K0tMR3MvVG1idTBrdGQ0cDQ4clRablgwd1JwVXZRUHZTellma3V5Uk9GWFhISm9YSnBBVnNQeGxs
NytLUHIKelFPSlRwVUVUQzVKdFdvV1o5UzNneHRaaGRJcWNYdnRlUW91Y0ZnbVFGYXhaNmNvZWNC
K2dlaFlqMFhwUk1DVQpkYzUzakdWOWpyTUhPdllCb3IwM0I4N0tpVzgxWHF1eEFwY2NRcVlXQ1hW
K2RaT1lRaUJLdjNHZnJsR1lqUEpsCmNFL0JRMUxrczZOWDhPU2Q4QkQyTERzRi9MTWg0UUZDT2Ft
c0lUNDdPMXkxNktkV20xMTR4VlE3T3BYTkNzV1YKandCWnZUVFdWdTQxVkJKbjNiV3VxY1Rjei9E
eDQ1SXZ2aGZqbWhDbXVjN3NqemVwclVyUTlmN1U0dUY3eFB6RwpvMnBZaFYyczVtblFoUU9QMXZq
RjNhSjhneUQ5NGdQSCtFN3doWEZlZ2dJVWtvbE5VYVhlSUJrd0ZDV3dEdlZiCnpLVkhSWUJCOGcw
NmJFNTlYaVNza2lNY0MyenZrN21RTEZNNndwYklDN2h6WStmTkV4dGtPOWhJMTM1ajF0T3QKWG9X
L295RkQ1UDJTNXh5SWdyczBxdjF1OU9TU01Bd3BPYjd6UkZnUThaa0YvcVVsR1NpdzVUSHVRSDlO
M1N3NQpMWjJ1YjEybU1XbllkRU10K2diUVUzc1JOZnFHV1B3RnVsd3dhK3BBY0tXWWZySGhKVDg9
Cj1NdTBYCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo=

--===============0296245468072151377==--