Question about ancient TARGET_CFLAGS in rules.mk?

Christian Marangi ansuelsmth at gmail.com
Sun Jul 17 02:55:08 PDT 2022


On Sun, Jul 17, 2022 at 02:50:36AM -0700, Rosen Penev wrote:
> On Sun, Jul 17, 2022 at 2:29 AM Christian Marangi <ansuelsmth at gmail.com> wrote:
> >
> > On Sat, Jul 16, 2022 at 10:17:28PM -0700, Rosen Penev wrote:
> > > On Sat, Jul 16, 2022 at 4:52 PM Ansuel Smith <ansuelsmth at gmail.com> wrote:
> > > >
> > > > Hi,
> > > > some background about this.
> > > >
> > > > I'm trying to improve our CI system more and more by finally adding
> > > > support for real
> > > > EXTERNAL_TOOLCHAIN_SUPPORT... I'm running (and abusing) the github CI
> > > > to make sure everything works and all compiles correctly...
> > > >
> > > > While testing it I notice a specific target fails to compile ubox package.
> > > > While still to investigate why this is only present on that target, i
> > > > found out why
> > > > this happen with EXTERNAL_TOOLCHAIN and doesn't fail on a normal build.
> > > >
> > > > The error is this
> > > >
> > > > kmodloader.c: In function 'main_loader':
> > > > 1339kmodloader.c:1027:41: error: ignoring return value of 'asprintf'
> > > > declared with attribute 'warn_unused_result' [-Werror=unused-result]
> > > > 1340make[1]: *** [package/Makefile:116: package/system/ubox/compile] Error 1
> > > > 1341 1027 | asprintf(&m->opts, "%s %s", prev, opts);
> > > > 1342 | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > 1343cc1: all warnings being treated as errors
> > > >
> > > > The package is compiled with -Wall so it does make sense that the
> > > > error is printed...
> > > >
> > > > Fact is that the error(warning) is actually correct but I couldn't
> > > > understand why it was
> > > > not flagged on normal build and here the reason...
> > > >
> > > > In rules.mk we have
> > > > TARGET_CFLAGS+= -fhonour-copts -Wno-error=unused-but-set-variable
> > > > -Wno-error=unused-result
> > > > and this is only applied if EXTERNAL_TOOLCHAIN is not selected...
> > > >
> > > > Now the question... WHY?
> > > >
> > > > Considering even the linux kernel started to use Wall by default,
> > > > should we also start
> > > > enforcing correct code and fix every package that present such error?
> > > Yes
> >
> > Ok will prepare a patch.
> >
> > > > Fixing these kind of error is trivial enough and IMHO we should drop these
> > > > warning disable.
> > > I've had issues getting stuff merged to core openwrt utilities in the
> > > past, especially when it comes to fixing compilation warnings.
> >
> > Mhhh I notice sometime patch getting rejected as it was trying to fix an
> > false-positive error from a faulty version of gcc.
> > But I think fixing error caused by disabling warning should be
> > accepted... Real problem is that some trivial fix may cause problems...
> > Example the error i just fixed for kmodloader... If I wasn't carfule i
> > could totally check the error condition for (fail) instead of (fail < 0)
> > and that would have caused breakage as asprintf return the bytes written
> > so it could totally return a value != 0. (just an example of a simple
> > error handling destryong the function of the package)
> >
> > > >
> > > > I will create a PR in the next few days but wonder if anyone wants to
> > > > give some feedback
> > > > about why these extra flags are set. To me it seems they are just
> > > > leftover from older times?
> > > Most likely.
> >
> > Still can't understand why these errors are only in archs38/generic...
> > Still a mistery to me...
> very clear actually. glibc marks asprintf with
> attribute((warn_unused_result)) (nodiscard in C++). musl does not.

Ohhhhhhh! And archs38/generic use glibc NOW IT MAKES SENSE!
And it's the only target that use glibc toolchains...

Thanks a lot. Well time to use glibc instead of using github actions.

-- 
	Ansuel



More information about the openwrt-devel mailing list