Messages in this thread Patch in this message | | | Date | Mon, 9 Mar 2020 11:39:36 +0100 | From | Jessica Yu <> | Subject | Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface |
| |
+++ Masahiro Yamada [09/03/20 19:20 +0900]: >On Mon, Mar 9, 2020 at 6:59 PM Jessica Yu <jeyu@kernel.org> wrote: >> >> +++ Masahiro Yamada [09/03/20 09:40 +0900]: >> >Hi Jessica, >> > >> > >> > >> >On Sat, Mar 7, 2020 at 1:02 AM Jessica Yu <jeyu@kernel.org> wrote: >> >> >> >> Rework modpost's logging interface by consolidating merror(), warn(), and >> >> fatal() to use a single function, modpost_log(). Introduce different >> >> logging levels (WARN, ERROR, FATAL) as well. The purpose of this cleanup is >> >> to reduce code duplication when deciding whether or not to warn or error >> >> out based on a condition. >> >> >> >> Signed-off-by: Jessica Yu <jeyu@kernel.org> >> >> --- >> >> v3: >> >> - remove level variable from modpost_log and just call fprintf in each >> >> case >> >> - remove warn_unless and just call modpost_log() directly >> >> - fix checkpatch error: >> >> ERROR: space required before the open parenthesis '(' >> >> #102: FILE: scripts/mod/modpost.c:61: >> >> + switch(loglevel) { >> >> >> >> scripts/mod/modpost.c | 68 ++++++++++++++++++++++----------------------------- >> >> scripts/mod/modpost.h | 14 ++++++++--- >> >> 2 files changed, 40 insertions(+), 42 deletions(-) >> >> >> >> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c >> >> index 7edfdb2f4497..a2329235a6db 100644 >> >> --- a/scripts/mod/modpost.c >> >> +++ b/scripts/mod/modpost.c >> >> @@ -51,41 +51,34 @@ enum export { >> >> >> >> #define MODULE_NAME_LEN (64 - sizeof(Elf_Addr)) >> >> >> >> -#define PRINTF __attribute__ ((format (printf, 1, 2))) >> >> +#define PRINTF __attribute__ ((format (printf, 2, 3))) >> >> >> >> -PRINTF void fatal(const char *fmt, ...) >> >> +PRINTF void modpost_log(enum loglevel loglevel, const char *fmt, ...) >> >> { >> > >> > >> >This series looks good to me. >> > >> >I can queue it up to kbuild tree >> >if there is no objection. >> > >> > >> >I just noticed one nit. >> > >> >Now that modpost_log() is the only user of PRINTF, >> >we can delete PRITNF, and directly add the attribute >> >to modpost_log(), like this: >> > >> > >> >void __attribute__((format(printf, 2, 3))) >> >modpost_log(enum loglevel loglevel, const char *fmt, ...) >> >{ >> > ... >> >} >> > >> > >> >If you agree, I can modify it when I apply it. >> >> Yes, I agree with this change. Thank you! >> >> One more thing, it's not immediately obvious to me why the first patch >> would cause those kbuild warnings :-/ I'll see if I have any luck >> reproducing them locally.. >> > > >Hmm, this warning option is not so new. > > >At least, commit 7c0d35a339db6 (4 years ago) >fixed a similar one.
Ah, sorry, I mean the kbuild 0-day bot errors. I am just realizing the 0-day bot emails are not CC'd to lkml. Here is the error I got from the bot:
---
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master] [also build test ERROR on v5.6-rc4 next-20200306] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Jessica-Yu/modpost-rework-and-consolidate-logging-interface/20200307-052346 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 30fe0d07fd7b27d41d9b31a224052cc4e910947a config: sh-randconfig-a001-20200306 (attached as .config) compiler: sh4-linux-gcc (GCC) 7.5.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.5.0 make.cross ARCH=sh
If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> ERROR: modpost: "adc_single" [arch/sh/boards/mach-hp6xx/hp6xx_apm.ko] undefined!
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> >I use GCC 7.4 > >If I apply your previous version, >I see build log like follows: > > > > >$ gcc --version >gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0 >Copyright (C) 2017 Free Software Foundation, Inc. >This is free software; see the source for copying conditions. There is NO >warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > >masahiro@pug:~/ref/linux$ make defconfig all > HOSTCC scripts/kconfig/conf.o > HOSTCC scripts/kconfig/confdata.o > HOSTCC scripts/kconfig/expr.o > LEX scripts/kconfig/lexer.lex.c > YACC scripts/kconfig/parser.tab.[ch] > HOSTCC scripts/kconfig/lexer.lex.o > HOSTCC scripts/kconfig/parser.tab.o > HOSTCC scripts/kconfig/preprocess.o > HOSTCC scripts/kconfig/symbol.o > HOSTCC scripts/kconfig/util.o > HOSTLD scripts/kconfig/conf >*** Default configuration is based on 'x86_64_defconfig' ># ># configuration written to .config ># > SYSTBL arch/x86/include/generated/asm/syscalls_32.h > SYSHDR arch/x86/include/generated/asm/unistd_32_ia32.h > SYSHDR arch/x86/include/generated/asm/unistd_64_x32.h > SYSTBL arch/x86/include/generated/asm/syscalls_64.h > SYSHDR arch/x86/include/generated/uapi/asm/unistd_32.h > SYSHDR arch/x86/include/generated/uapi/asm/unistd_64.h > SYSHDR arch/x86/include/generated/uapi/asm/unistd_x32.h > HOSTCC arch/x86/tools/relocs_32.o > HOSTCC arch/x86/tools/relocs_64.o > HOSTCC arch/x86/tools/relocs_common.o > HOSTLD arch/x86/tools/relocs > HOSTCC scripts/selinux/genheaders/genheaders > HOSTCC scripts/selinux/mdp/mdp > HOSTCC scripts/kallsyms > HOSTCC scripts/sorttable > HOSTCC scripts/asn1_compiler > HOSTCC scripts/extract-cert > WRAP arch/x86/include/generated/uapi/asm/bpf_perf_event.h > WRAP arch/x86/include/generated/uapi/asm/errno.h > WRAP arch/x86/include/generated/uapi/asm/fcntl.h > WRAP arch/x86/include/generated/uapi/asm/ioctl.h > WRAP arch/x86/include/generated/uapi/asm/ioctls.h > WRAP arch/x86/include/generated/uapi/asm/ipcbuf.h > WRAP arch/x86/include/generated/uapi/asm/param.h > WRAP arch/x86/include/generated/uapi/asm/poll.h > WRAP arch/x86/include/generated/uapi/asm/resource.h > WRAP arch/x86/include/generated/uapi/asm/socket.h > WRAP arch/x86/include/generated/uapi/asm/sockios.h > WRAP arch/x86/include/generated/uapi/asm/termbits.h > WRAP arch/x86/include/generated/uapi/asm/termios.h > WRAP arch/x86/include/generated/uapi/asm/types.h > WRAP arch/x86/include/generated/asm/early_ioremap.h > WRAP arch/x86/include/generated/asm/export.h > WRAP arch/x86/include/generated/asm/mcs_spinlock.h > WRAP arch/x86/include/generated/asm/mm-arch-hooks.h > WRAP arch/x86/include/generated/asm/mmiowb.h > WRAP arch/x86/include/generated/asm/dma-contiguous.h > UPD include/config/kernel.release > UPD include/generated/uapi/linux/version.h > UPD include/generated/utsrelease.h > CC scripts/mod/empty.o > HOSTCC scripts/mod/mk_elfconfig > MKELF scripts/mod/elfconfig.h > HOSTCC scripts/mod/modpost.o >scripts/mod/modpost.c: In function ‘modpost_log’: >scripts/mod/modpost.c:75:2: warning: format not a string literal and >no format arguments [-Wformat-security] > fprintf(stderr, level); > ^~~~~~~ > CC scripts/mod/devicetable-offsets.s > UPD scripts/mod/devicetable-offsets.h > HOSTCC scripts/mod/file2alias.o > HOSTCC scripts/mod/sumversion.o > HOSTLD scripts/mod/modpost > CC kernel/bounds.s > UPD include/generated/bounds.h > UPD include/generated/timeconst.h > CC arch/x86/kernel/asm-offsets.s > > >-- >Best Regards >Masahiro Yamada
| |