From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 15AA75A026F for ; Wed, 27 Dec 2023 21:25:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1703708721; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=af5Fhniqr+f2UkVrNpVkbuEJIBCm3ZBPzPeydk3H5L8=; b=fsKzywEz2KLc4wLcKsAG+HjjaVxTwCEcg1/TD+gVnj53urPF79kvqD/g0sBYF2vsyetrHJ V4Fr0K2KDD2uCp2SxTBSfCzhGaHpEmJTJw0lUEnLwzhDPu5/nXwCfoT2puKlowsqmcS2tE qJsUWoyxX8jZzv2XSr8YJ4p/eGopTWA= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-606-6qn33OzbN1et9OK5Qy_7eA-1; Wed, 27 Dec 2023 15:25:19 -0500 X-MC-Unique: 6qn33OzbN1et9OK5Qy_7eA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1ABFA83106C; Wed, 27 Dec 2023 20:25:19 +0000 (UTC) Received: from elisabeth (unknown [10.39.208.24]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 493682026D66; Wed, 27 Dec 2023 20:25:18 +0000 (UTC) Date: Wed, 27 Dec 2023 21:25:15 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 5/8] util: Improve sockaddr initialisation in sock_l4() Message-ID: <20231227212515.04a42717@elisabeth> In-Reply-To: <20231207143140.1851378-6-david@gibson.dropbear.id.au> References: <20231207143140.1851378-1-david@gibson.dropbear.id.au> <20231207143140.1851378-6-david@gibson.dropbear.id.au> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: CCY4BZPIJ4AQJLTFYVLLKLMFUNYY2SA4 X-Message-ID-Hash: CCY4BZPIJ4AQJLTFYVLLKLMFUNYY2SA4 X-MailFrom: sbrivio@redhat.com 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: On Fri, 8 Dec 2023 01:31:37 +1100 David Gibson wrote: > 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 explicit > initialiser macros for the address. > > 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. > > Signed-off-by: David Gibson > --- > util.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > 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 = { > .sin_family = AF_INET, > .sin_port = htons(port), > - { 0 }, { 0 }, > + .sin_addr = IN4ADDR_ANY_INIT, The second { 0 } was meant to initialise .sin_zero, and: > }; > struct sockaddr_in6 addr6 = { > .sin6_family = AF_INET6, > .sin6_port = htons(port), > - 0, IN6ADDR_ANY_INIT, 0, > + .sin6_addr = IN6ADDR_ANY_INIT, these zeroes were for sin6_flowinfo and sin6_scope_id. 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. 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. :( 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. 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. 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. The rest of the series looks good to me and I just applied it (minus _this part_ of this patch). -- Stefano