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 1/8] Improve types and names for port forwarding configuration
Date: Sat, 24 Sep 2022 12:57:25 +1000
Message-ID: <Yy5yFVvyQD0QkvtT@yekko>
In-Reply-To: <20220924005257.0c325e20@elisabeth>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============4346360085109929675=="

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

On Sat, Sep 24, 2022 at 12:52:57AM +0200, Stefano Brivio wrote:
> On Fri, 23 Sep 2022 14:57:59 +1000
> David Gibson <david(a)gibson.dropbear.id.au> wrote:
>=20
> > [...]
> >=20
> > --- /dev/null
> > +++ b/port_fwd.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: AGPL-3.0-or-later
> > + * Copyright Red Hat
> > + * Author: Stefano Brivio <sbrivio(a)redhat.com>
> > + * Author: David Gibson <david(a)gibson.dropbear.id.au>
> > + */
> > +
> > +#ifndef PORT_FWD_H
> > +#define PORT_FWD_H
> > +
> > +enum port_fwd_mode {
> > +	FWD_SPEC =3D 1,
> > +	FWD_NONE,
> > +	FWD_AUTO,
> > +	FWD_ALL,
> > +};
> > +
> > +typedef uint8_t port_fwd_map[DIV_ROUND_UP(USHRT_MAX, 8)];
>=20
> Given that this gets conveniently embedded in a struct in 2/8, could we
> avoid the typedef (or perhaps drop it after 2/8)? It makes the actual
> type less obvious to figure out, and in general I agree with most
> points from this slide deck:
>=20
>   http://www.kroah.com/linux/talks/ols_2002_kernel_codingstyle_talk/html/mg=
p00025.html

As do I, in fact especially for array types, due to their
idiosyncratic handling in C.

> :) ...well, unless there's some resulting complexity I'm missing.

Well, maybe.  I added the typedef because of two things: 1) the fact
that in one place we use a bitmap of this format separately from the
rest for the 'exclude' map in conf_ports().  2) the fact that we need
the size of this map in get_bound_ports().  Having the typedef lets us
handle both without duplicating the calculation of the size, which
would mean more opportunities to get it wrong.

I feel the advantages outweigh the disadvantages of a typedef in this
case, but I won't be offended if you disagree.  We could use a #define
or const of the bitmap size instead.

> I reviewed the rest of the series, it all makes sense to me, thanks.

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

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

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUVCQ0FBZEZpRUVvVUx4V3U0L1dz
MGRCK1h0Z3lwWTRnRXdZU0lGQW1NdWNnMEFDZ2tRZ3lwWTRnRXcKWVNLdEZ4QUFrTmE3cnYxYmg2
TVVEaXcrOHhIVzdoY3VMMWNoNnI3aGdOSU1qaFViWWtIOEJCWGFIYXovbGZYLwp4eC8rN09MQVVO
S2xXb29FaWQ2eFNobnovODQxeUd5a2twM2hPY2p3UW91NHc2V1FrclBtOWdaWWloQUU0N0RBCktK
VEg5d2Z3UWRxOHNVWnNwZmh0eUlXekx2aDFoSXp5TUFvd1JDZDVibDkrZExST0VjdHpjUDhKZHRI
dEZKdXcKT3ZZY2VNeGF2M0JSSURtelNkcTZ0Q0ZNMUNQQUxmQkZLV1hnVnBKbGpsVjVTMXpGSmto
M2FwdVh1T3Q4clB0SQptZHdiNXE3bi9xMFJ1UHpJNlJ5SnlVTHpEbE0wd1VPSm44Y2VIVWRBWmRH
c0J0YUY4bDJwV052SkgzOUR2YllwCldNQkJ0MEp4dkEvdFZabUgycTJPbHhtcUlzWHNMUkExeUVB
ejc0TFgreWkrU2J2M2ZQdU12U2IrUUk1VEMwcWEKRzdyUVcxMTEvZ01Eak43N1JIeko2NlNKQUJD
a0RpVm9kNTRzV3JLNkVwM1VoOWt3MWhlTHhJNUZlQmZUZmdndApEMEVvL3EyMG0xbm9wUUgyY3NU
NkpqYTFYSUYzTDlhSmhaT2RMU3RDeEl4dW9RTC9EamZxaFd3WGx1VldKWmNNCjl1ZFlzeXlHWWtZ
VVU2cUthekRZOVZRYU5MQmNOMDdPZ0lHejlsY0NmLzErT2ovVFM3aGVNOUZlc25saUU3VngKb3Ay
SFRNb2MxM01FeCtXRlN0THBVNjU1Y2VrQXg2Z0RyN01nME84eDFOSFVYL0xkSmxJYzR0N01jUkti
cWd2ZgpTLzRYd3BxVVdmSDhHVE5RSGVocjhOM3oyMmE4aEdxOEJaVVJiRXhNT2U2cUorc3BEb2s9
Cj03VTdSCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo=

--===============4346360085109929675==--