From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Brivio To: passt-dev@passt.top Subject: Re: [PATCH 01/28] Clean up parsing of port ranges Date: Wed, 28 Sep 2022 22:57:31 +0200 Message-ID: <20220928225731.124f76d4@elisabeth> In-Reply-To: <20220928043339.613538-2-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0044219438118906236==" --===============0044219438118906236== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit On Wed, 28 Sep 2022 14:33:12 +1000 David Gibson wrote: > 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. > > Rework the code with the use of two new helper functions: > * parse_port_range() operates a bit like strtoul(), but can parse a whole > 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 > > The new version is easier to follow, Indeed. Just one excess whitespace: > [...] > + if (*p == ':') { /* 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) != > + (orig_range.last - orig_range.first)) > goto bad; > + } else { > + mapped_range = orig_range; > + } > > - if (end_dst != -1 && > - end_dst - start_dst != end_src - start_src) > - goto bad; /* 22-81:8022:8080 */ > - > - for (i = start_src; i <= end_src; i++) { > - if (bitmap_isset(fwd->map, i)) > - goto overlap; > + if ((*p != '\0') && (*p != ',')) /* Garbage after the ranges */ ...here. I can drop it myself. -- Stefano --===============0044219438118906236==--