On Sat, Jul 16, 2022 at 10:17:28PM -0700, Rosen Penev wrote: > On Sat, Jul 16, 2022 at 4:52 PM Ansuel Smith <ansuels...@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@lists.openwrt.org > > https://lists.openwrt.org/mailman/listinfo/openwrt-devel -- Ansuel _______________________________________________ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel