Messages in this thread Patch in this message | | | Date | Mon, 9 Mar 2020 10:59:14 +0100 | From | Jessica Yu <> | Subject | Re: [PATCH v3 1/2] modpost: rework and consolidate logging interface |
| |
+++ 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..
Thanks,
Jessica
| |