On Mon, Sep 21, 2020 at 11:51 PM Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote: > > Thanks for the review, > > >> Set KBUILD_MODPOST_FAIL_ON_WARNINGS to a non-empty value to make the > >> kbuild fail when modpost generates any warnings. This will avoid > >> misses such as [1] where the SOF CI did not catch a missing module > >> license. > >> > >> This was initially contributed in 2016 [2], rebase/clean-ups and tests > >> by Pierre Bossart. > >> > >> Test example: > >> $ KBUILD_MODPOST_FAIL_ON_WARNINGS=1 make > >> GEN Makefile > >> DESCEND objtool > >> CALL sof-dev/scripts/atomic/check-atomics.sh > >> CALL sof-dev/scripts/checksyscalls.sh > >> CHK include/generated/compile.h > >> MODPOST Module.symvers > >> Kernel: arch/x86/boot/bzImage is ready (#13) > >> WARNING: modpost: missing MODULE_LICENSE() in sound/soc/intel/boards/snd-soc-sof-sdw.o > >> make[2]: *** [sof-dev/scripts/Makefile.modpost:114: Module.symvers] Error 2 > > > > > > I think [1] should be an error instead of a warning > > by default. > > would the following patch be what you have in mind? No. error() does not exist. merror() exists, but the difference from warn() is just a prefix. If any error happens, modpost should return the error code. > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 422f1cfca289..ae1eb67aa0f2 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -2018,7 +2018,7 @@ static void read_symbols(const char *modname) > if (!mod->is_vmlinux) { > license = get_modinfo(&info, "license"); > if (!license) > - warn("missing MODULE_LICENSE() in %s\n", modname); > + error("missing MODULE_LICENSE() in %s\n", modname); > while (license) { > if (license_is_gpl_compatible(license)) > mod->gpl_compatible = 1; > > > If yes, also wondering if we can still add the option to treat warnings > as errors as an opt-in behavior? I want to add a new option only when it is necessary to do so. I am not sure which warnings are real warnings. > Thanks! > -Pierre -- Best Regards Masahiro Yamada