From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202602 header.b=PdjqyChG; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 35F9F5A0262 for ; Fri, 15 May 2026 03:21:21 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1778808078; bh=98tyMSQaB+NJecAxgqaCv9WFBRc/9owPw8uO8aA32bY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PdjqyChGOCrpnD2CYPU3KPmqsTe/80dil9+/Ij85iy1tkMr4qM8EQcwt1C81sO7ch F4lTUBJtXla4mdzsCT8uXYrlNfvGh45hGBHrwfnD2ic9e0IuTKQHVSg3WEtnA0zqIv JWY02+I661rxfS26V/wEftDiSds0Txmhu+KTCFfnkKOKCY7WymCcSXhgjKzgD9FgUf f1cmqkwofcbbK48aAMlFOP0kQRqB+Dnt+SP5OzsKn7k5/avGxwTyXH0aFBsv7Amfz9 x1iNRFiK3LotEMZXLHmkm14zx+x2krGwxE1djSfRuY+dLTWpep4llwy4YWlMq0/HpZ a9MZvO3rqe9xA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gGqCf2cPbz4wTh; Fri, 15 May 2026 11:21:18 +1000 (AEST) Date: Fri, 15 May 2026 10:59:28 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 06/12] Makefile: Split $(FLAGS) into cpp and cc components Message-ID: References: <20260512055256.1800449-1-david@gibson.dropbear.id.au> <20260512055256.1800449-7-david@gibson.dropbear.id.au> <20260513091059.051d41c5@elisabeth> <20260514114102.294abd87@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="7VQCfUZiLFizsmMB" Content-Disposition: inline In-Reply-To: <20260514114102.294abd87@elisabeth> Message-ID-Hash: 72Q4IIAHUJW2BBDOV3PD7P37FZBPMYDP X-Message-ID-Hash: 72Q4IIAHUJW2BBDOV3PD7P37FZBPMYDP 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: --7VQCfUZiLFizsmMB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, May 14, 2026 at 11:41:02AM +0200, Stefano Brivio wrote: > On Thu, 14 May 2026 12:01:24 +1000 > David Gibson wrote: >=20 > > On Wed, May 13, 2026 at 09:11:00AM +0200, Stefano Brivio wrote: > > > On Tue, 12 May 2026 15:52:50 +1000 > > > David Gibson wrote: > > > =20 > > > > The $(FLAGS) variable contains mandatory compiler flags that should= not be > > > > overridden. However, it contains a mixture of flags for the prepro= cessor > > > > and for the compiler proper. That's causing some inconvenience for= other > > > > Makefile cleanups, so split it into $(BASE_CPPFLAGS) and $(BASE_CFL= AGS) > > > > variables. > > > >=20 > > > > Signed-off-by: David Gibson > > > > --- > > > > Makefile | 40 ++++++++++++++++++++++++---------------- > > > > 1 file changed, 24 insertions(+), 16 deletions(-) > > > >=20 > > > > diff --git a/Makefile b/Makefile > > > > index a8f8d06e..697f229f 100644 > > > > --- a/Makefile > > > > +++ b/Makefile > > > > @@ -30,12 +30,17 @@ ifeq ($(shell $(CC) -O2 -dM -E - < /dev/null 2>= &1 | grep ' _FORTIFY_SOURCE ' > / > > > > FORTIFY_FLAG :=3D -D_FORTIFY_SOURCE=3D2 > > > > endif > > > > =20 > > > > -FLAGS :=3D -Wall -Wextra -Wno-format-zero-length -Wformat-security > > > > -FLAGS +=3D -pedantic -std=3Dc11 -D_XOPEN_SOURCE=3D700 -D_GNU_SOURCE > > > > -FLAGS +=3D $(FORTIFY_FLAG) -O2 -pie -fPIE > > > > -FLAGS +=3D -DPAGE_SIZE=3D$(shell getconf PAGE_SIZE) > > > > -FLAGS +=3D -DVERSION=3D\"$(VERSION)\" > > > > -FLAGS +=3D -DDUAL_STACK_SOCKETS=3D$(DUAL_STACK_SOCKETS) > > > > +# Mandatory preprocessor flags that won't be overridden with $(CPP= FLAGS) > > > > +# FIXME: Could some of these be default, rather than required? > > > > +BASE_CPPFLAGS :=3D -D_XOPEN_SOURCE=3D700 -D_GNU_SOURCE $(FORTIFY_F= LAG) > > > > +BASE_CPPFLAGS +=3D -DPAGE_SIZE=3D$(shell getconf PAGE_SIZE) > > > > +BASE_CPPFLAGS +=3D -DVERSION=3D\"$(VERSION)\" > > > > +BASE_CPPFLAGS +=3D -DDUAL_STACK_SOCKETS=3D$(DUAL_STACK_SOCKETS) > > > > + > > > > +# Mandatory compiler flags that won't be overridden with $(CFLAGS) > > > > +# FIXME: Could some of these be default, rather than required? > > > > +BASE_CFLAGS :=3D -std=3Dc11 -pie -fPIE -O2 > > > > +BASE_CFLAGS +=3D -pedantic -Wall -Wextra -Wno-format-zero-length -= Wformat-security =20 > > >=20 > > > This new version of the series looks good to me in general (minus > > > potential concern reported below), and everything seems to work on > > > Debian and Fedora, but I would still like to try things out on Alpine > > > or Void Linux because musl might cause surprises. I haven't got to it > > > yet. =20 > >=20 > > Makes sense, though I'm pretty sure it will be safe. I considered > > having some later patches that started removing non-essential things > > from BASE_*FLAGS. Those would certainly have required careful testing > > across distros. But working out what should move became more > > complicated than I initially thought (that's basically the discussion > > later in this mail), so I postponed it. >=20 > If you plan to submit another series soon, note that I don't plan to > re-do the testing I'm currently doing, though, so you should plan to do > those yourself and work with maintainers if things break, in case. Understood, but I don't plan to resume this particularly soon. > > > Meanwhile, regarding these FIXME comments: I think it *is* currently > > > possible to override those flags (with different values for the same > > > options), and overriding -D_FORTIFY_SOURCE on openSUSE (I haven't > > > tried right now) was the initial motivation behind FLAGS. =20 > >=20 > > True, I was thinking "overridden" in the make variable sense, but at > > the actual compiler that's misleading. > >=20 > > > That is, the overriding role of CFLAGS seems to be preserved for these > > > BASE_* flags as well, because $CFLAGS is given to the compiler after > > > $BASE_CPPFLAGS, $CPPFLAGS, and $BASE_CFLAGS. So, in this sense, I wou= ld > > > already call them "default" flags. =20 > >=20 > > Right, it's kind of the subtle distinction between two types of > > default: default unless that specific option is overridden > > (BASE_CFLAGS), or default unless all the "default" options are > > overridden as a block (CFLAGS). > >=20 > > > If that's the case, I think it's fine. Otherwise we need to find > > > another solution at least for the short term. > > >=20 > > > By the way, if it helps addressing those comments at some point (I > > > would apply anyway this series meanwhile if I don't find breakages, > > > because not being able to run static checkers automatically on pesto = is > > > pretty nasty), out of those flags: > > >=20 > > > * -D_XOPEN_SOURCE, -D_GNU_SOURCE, and -DPAGE_SIZE are strictly requir= ed > > > to build (at least in some environments) =20 > >=20 > > Yes. > >=20 > > > * -D_FORTIFY_SOURCE, -pie, -fPIE are not required to build but they a= re > > > critical for security =20 > >=20 > > Right. I'd kind of consider -pie and -fPIE in a different category > > than "flags", since they're choosing the kind of output object we're > > generating. > >=20 > > > * -DVERSION is not required to build but makes things confusing and > > > issues hard to debug because the version (usually supplied by the > > > distribution) isn't reported in logs and logs of other tools =20 > >=20 > > Right. I guess I'd also consider it mandatory. The fact that you can > > sort of get away without it seems more an accident of how we include > > it, rather than policy. > >=20 > > > - -DDUAL_STACK_SOCKETS doesn't seem to be used anymore starting from > > > commit b8d4fac6a2e7 ("util, pif: Replace sock_l4() with > > > pif_sock_l4()")... was it intended, actually? =20 > >=20 > > Huh, good point. I'm not sure it was exactly intentional. I was > > certainly grappling with the fact that in a sense it sometimes wanted > > to return two sockets in the !DUAL_STACK_SOCKETS case was really > > awkward. My plan, as best I recall was to move the handling of that > > case up layers, until it reached what would eventually become the > > forward table - so separate forwarding entries for v4 and v6 if we > > don't have dual stack sockets. > >=20 > > But, since we introduced DUAL_STACK_SOCKETS, I've discovered that the > > dual stack socket interface is described by RFC and supported by BSD > > and Windows as well as Linux. I'm pretty happy killing this define. > >=20 > > > - -std=3Dc11 is strictly required to ensure we build things correctly= =20 > >=20 > > Huh, is it? I would have thought it just catches us from using any > > C23 or GNU extensions. >=20 > It is because, at least in theory, while some constructs we use might > be valid C99 or C17, we don't have guarantees that they have the same > semantics. I don't see it happening in practice but I wouldn't risk it. I mean... the C committee has done some weird stuff, but changing the behaviour of existing valid constructs seems well beyond the pale. > Also mind that building with -std=3Dc99 doesn't work (anymore). Certainly - at this point it's a constraint that CC refer to a C11 compiler, not just a C compiler. > > > - -O2 is optional, but dropping it (by default) might require > > > annoying adjustments in distributions =20 > >=20 > > Right. But on the other hand, overriding "all" packages with the > > distro preferred optimization flags is one of the main uses for distro > > CFLAGS overrides. Not sure what to do about this one. >=20 > Leave it like it is because it works as intended. It's our default > which is also the default for most distributions, and Gentoo users > (it's pretty much about them) can still override it by passing -O3 > later. Yeah, ok. > > > - -pedantic, -Wall, -Wextra, -Wno-format-zero-length, -Wformat-securi= ty > > > are all optional and useful for development (including distribution > > > development), and might be security relevant in some cases =20 > >=20 > > They're security relevant in the sense that it's good for us to get > > the warnings to catch possible security bugs. But a tree that builds > > clean with the flags shouldn't have different security properties if > > it's built without them. So I think it only matters that *we* use the > > flags, we don't need to impose it on anything else. >=20 > See above: "(including distribution development)". If those aren't > there, we won't get some warnings from building distribution packages, > and a few of those were useful in the past. >=20 > > But, there's an awkward gotcha here. The build.py testcase needs > > -Werror to catch warnings, but obviously we don't want it to have to > > reiterate all the other warning flags. If we put these all into > > CFLAGS, it's tricky for build.py to just add -Werror without touching > > the rest. >=20 > My whole point was / is that CFLAGS needs to *add* to the existing > flags, including warnings, instead of replacing them. You can override > warnings flags too by appending options. >=20 > > From our point of view, I like the idea of adding -Werror > > to the default CFLAGS, which fixes that problem. >=20 > At the moment there's no problem though. >=20 > > But as you pointed > > out when I did that in the earlier version that might break distro > > builds causing use more trouble to fix it. >=20 > That *would* break distribution builds at the moment, so it's not > acceptable. I constantly have fixes for warnings I spot in the packages > I build on my to-do list. Yeah, fair points. --=20 David Gibson (he or they) | 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 --7VQCfUZiLFizsmMB Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmoGb+AACgkQzQJF27ox 2Gf+Vw//c82gj4Zzd2fLyrbqGCbWMzyO4D0hWhp/VE+mpJxiQYDVxPaN7e4JIg+k OnDeyla3R1cQjkhiOvCFsap9jB2fHSjZMVC2rd8JsTT+sH6v6xlaGuSm+N/6q+rl 1ni27uacFiGDonJhEZjHCZSV/pyZ46wPJnBWvU35dG1D6jiIdPoyMamrQ4yQ6RQI eM+BtWPiKyVZ34WRmNcZKT+m5+MYR2hkqXk4M7GABMJm7mmN/QreeeYhxki7WZaB Nic2w/8/xripB17RCFp1CM4vSyjnSC1AnbFJV55XpPXvN2KNGO6VT5uqT+r1mnpM wtL10x4kpeNe7MpldhZBF4rhLQsOf4bu+AbwtE404N5tlqi+vQ7AjkuTKaipf38s AkkRX4W0MKRN0Fmh8BVgFZAi/ZBTkcaC3q0W8KYQRIVimJBSOMnOSFHLZ9s6/ElR gfC7AvWX3kvuJ+sMpBdt9U3cIShXDV0fqc2eRbpzn1Q+VwQICldW/H/LeTZ8pn1B i/VLfet0wvzcgbY51BrBWmVTzfzXN4+dUGvxHBTZsaxq06abMkwbNs7xzv06GK9i mfcl3qCyH+REO2xf187YUlFUyS+prgjIlt57+77sjyrVKAJg+TgY3YlSi47QSnp3 wLPwm/lkoC/ncP+nnMNHM3KPnsGwPqlwVHoan+b6s0wUdwwzzDM= =oxYa -----END PGP SIGNATURE----- --7VQCfUZiLFizsmMB--