On Sun, Jul 17, 2022 at 2:29 AM Christian Marangi <ansuels...@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 <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... very clear actually. glibc marks asprintf with attribute((warn_unused_result)) (nodiscard in C++). musl does not. > > > > > > > _______________________________________________ > > > 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
