From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 1A5985A026F for ; Sun, 7 Jan 2024 06:36:11 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1704605765; bh=dAJwxBGuVtcBFnsP8j2UUP0avup5MXsB1GfZ9z3rZOs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=nF5xz5f2IT9h8TvmJgWhbOrbC1/R22wXfOtkVndW/7KbqPQx5GlIFqsVByA6nmQLL Rd/Ekw40y29LRTY7G9rXg7RV71tRGldoMHhVnfZNOMVnF4gP6D1QcSwmQT97wMzEXQ DQEwPRRpwiG3sOBDD8WY6wKW2B/RqHO+hSKqtyKY7Hg+H6YAncj9Chyz5GQXnWkhMO FMwHtrJRmi5s6iyrNgJRXicMQ4Fd7BfYQQj3kVFLvQttCtDdy/2P33FYpx6mEnRf4i Ld8FCSluBMXVkLXCjQo/WWzEmm5hnK8u1uardhurxPdyvB8ocFO58z8isgrwBKEHEg rXFzkqd0a0mhQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4T75X51R9jz4wnw; Sun, 7 Jan 2024 16:36:05 +1100 (AEDT) Date: Sun, 7 Jan 2024 16:34:02 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 5/8] util: Improve sockaddr initialisation in sock_l4() Message-ID: References: <20231207143140.1851378-1-david@gibson.dropbear.id.au> <20231207143140.1851378-6-david@gibson.dropbear.id.au> <20231227212515.04a42717@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="aiW1Y11R5/xFltpg" Content-Disposition: inline In-Reply-To: <20231227212515.04a42717@elisabeth> Message-ID-Hash: ZRBG2ASPL4W2QAAL4JJHBTPQZHZIS4KH X-Message-ID-Hash: ZRBG2ASPL4W2QAAL4JJHBTPQZHZIS4KH X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --aiW1Y11R5/xFltpg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Dec 27, 2023 at 09:25:15PM +0100, Stefano Brivio wrote: > On Fri, 8 Dec 2023 01:31:37 +1100 > David Gibson wrote: >=20 > > Currently we initialise the address field of the sockaddrs we construct > > to the any/unspecified address, but not in a very clear way: we use > > explicit 0 values, which is only interpretable if you know the order of > > fields in the sockaddr structures. Use explicit field names, and expli= cit > > initialiser macros for the address. > >=20 > > Because we initialise to this default value, we don't need to explicitly > > set the any/unspecified address later on if the caller didn't pass an > > overriding bind address. > >=20 > > Signed-off-by: David Gibson > > --- > > util.c | 8 ++------ > > 1 file changed, 2 insertions(+), 6 deletions(-) > >=20 > > diff --git a/util.c b/util.c > > index d465e48..0152ae6 100644 > > --- a/util.c > > +++ b/util.c > > @@ -105,12 +105,12 @@ int sock_l4(const struct ctx *c, int af, uint8_t = proto, > > struct sockaddr_in addr4 =3D { > > .sin_family =3D AF_INET, > > .sin_port =3D htons(port), > > - { 0 }, { 0 }, > > + .sin_addr =3D IN4ADDR_ANY_INIT, >=20 > The second { 0 } was meant to initialise .sin_zero, and: >=20 > > }; > > struct sockaddr_in6 addr6 =3D { > > .sin6_family =3D AF_INET6, > > .sin6_port =3D htons(port), > > - 0, IN6ADDR_ANY_INIT, 0, > > + .sin6_addr =3D IN6ADDR_ANY_INIT, >=20 > these zeroes were for sin6_flowinfo and sin6_scope_id. >=20 > Not needed because we want zeroes anyway (or the "same as objects that > have static storage duration"), but see commit eed6933e6c29 ("udp: > Explicitly initialise sin6_scope_id and sin_zero in sockaddr_in{,6}"): > gcc versions 7 to 9 used to complain. And gcc 9 isn't *that* old. >=20 > But then, you might ask, why didn't I just use names for all the > initialisers? Well, there was some issue with sockaddr_in6 or > sockaddr_in not having a field defined in some header (kernel or a C > library). I have a vague memory it was about sin6_scope_id, but I can't > find any note in the git history or anywhere else. :( >=20 > Now, the latest addition to sockaddr_in6 was sin6_scope_id (kernel: > 2006, glibc: 2000), so that's fine. Also sockaddr_in looks okay, > including sin_zero, with "#define sin_zero __pad" in the kernel version. >=20 > So... my preferred option would be to leave this untouched: the > initialisers you replaced are anyway all zeroes. And, after RFC 2553 > (24 years ago), the order of those fields never changed. >=20 > If it really bothers you, let's at least initialise everything > explicitly by name, because we know that gcc 9 complains, and if we hit > another version of sockaddr_in6 without a named sin6_scope_id, we'll > find out, eventually. >=20 > The rest of the series looks good to me and I just applied it (minus > _this part_ of this patch). Eh, ok. I'll worry about this bit if I run across it again. --=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 --aiW1Y11R5/xFltpg Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmWaN8kACgkQzQJF27ox 2Gd2sA/+LdiCZFUa8kd5+hM72r2Ue3RBBfX8wfy/G4AykFMKh57OBgbF9W8hZ8cr SnkTPG2wdP5cYFQ7IMLog604YyJWPDqaZnsyz3wW71Lf2RxNbVVzIorrU8O04D0w OCi4i9/w0q+9V5AWaNj0j5d/SAqrRIZLZuKRzPnwNK24e0Jn5ueQppZNEgezBwcV D5JnotGM4o14jyk2HLF/kmC3RIdtjNDxrHmYtHL/zIX1gcniFiyYIuT/eenvrB9u EB4O9yJlxxRVKPf6yZA4kGkBfRL8BvHKvGYFuHtKBZLRK0aywJpOwQuG6fYgX0ib LtpK9lIc2l3qnB9I8sXq/6BjsbapdV7qqQSFvSotGsmWXCSdu79pXARSZ0ERCPsX h69kYFqGtJpnbOlOxNtlE7TiAdC1DUsEZSL5MvcCjb4+xpqUfEliRwbNUqIpq6Pm H0gxJPLLVsusZYqH4Ao4K4b7Ie56vArOPNG+5Tla0cejEua20bkUKhVH92SvxhNI pjZODC8/6fbB1h0+5Z0H64uQL1lpIILf4XlIK0tK9nwW1PhmyjUo7ZEzbPz2HWpU WcmwuBiLNofvboh+jgM005uuqS+Vap5VeKNDt2BKpc3llTEonZZGc+fSufZT3SAl 2n9lwXq6wyx30B0i4caIKCBxzMbc36UvrErU/UOpoX3/A1PtK6I= =iMNj -----END PGP SIGNATURE----- --aiW1Y11R5/xFltpg--