Question about ancient TARGET_CFLAGS in rules.mk?

[Christian Marangi]

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

> >
> > _______________________________________________
> > openwrt-devel mailing list
> > openwrt-devel at lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel

-- 
	Ansuel