Messages in this thread Patch in this message | | | From | Jessica Yu <> | Subject | [PATCH 1/2] modpost: rework and consolidate logging interface | Date | Tue, 25 Feb 2020 18:35:25 +0100 |
| |
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 as a conditional warn (warn_unless()). The conditional warn is useful in determining whether to use merror() or warn() based on a condition. This reduces code duplication overall.
Signed-off-by: Jessica Yu <jeyu@kernel.org> --- Hi,
This patch was added in response to the code duplication comments in the first version of "modpost: return error if module is missing ns imports and MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS=n" [1]
[1] See 20200214143709.6490-1-jeyu@kernel.org
scripts/mod/modpost.c | 69 ++++++++++++++++++++++++--------------------------- scripts/mod/modpost.h | 22 +++++++++++++--- 2 files changed, 51 insertions(+), 40 deletions(-)
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 7edfdb2f4497..e2805fbeb99f 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -51,41 +51,39 @@ 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, ...) { + char *level = NULL; va_list arglist; - fprintf(stderr, "FATAL: "); + switch(loglevel) { + case(LOG_WARN): + level = "WARNING: "; + break; + case(LOG_ERROR): + level = "ERROR: "; + break; + case(LOG_FATAL): + level = "FATAL: "; + break; + default: /* invalid loglevel, ignore */ + break; + } - va_start(arglist, fmt); - vfprintf(stderr, fmt, arglist); - va_end(arglist); + if (level) + fprintf(stderr, level); - exit(1); -} - -PRINTF void warn(const char *fmt, ...) -{ - va_list arglist; - - fprintf(stderr, "WARNING: "); + fprintf(stderr, "modpost: "); va_start(arglist, fmt); vfprintf(stderr, fmt, arglist); va_end(arglist); -} - -PRINTF void merror(const char *fmt, ...) -{ - va_list arglist; - fprintf(stderr, "ERROR: "); + if (loglevel == LOG_FATAL) + exit(1); - va_start(arglist, fmt); - vfprintf(stderr, fmt, arglist); - va_end(arglist); } static inline bool strends(const char *str, const char *postfix) @@ -113,7 +111,7 @@ static int is_vmlinux(const char *modname) void *do_nofail(void *ptr, const char *expr) { if (!ptr) - fatal("modpost: Memory allocation failure: %s.\n", expr); + fatal("Memory allocation failure: %s.\n", expr); return ptr; } @@ -2021,7 +2019,7 @@ static void read_symbols(const char *modname) license = get_modinfo(&info, "license"); if (!license && !is_vmlinux(modname)) - warn("modpost: missing MODULE_LICENSE() in %s\n" + warn("missing MODULE_LICENSE() in %s\n" "see include/linux/module.h for " "more information\n", modname); while (license) { @@ -2152,15 +2150,15 @@ static void check_for_gpl_usage(enum export exp, const char *m, const char *s) switch (exp) { case export_gpl: - fatal("modpost: GPL-incompatible module %s%s " + fatal("GPL-incompatible module %s%s " "uses GPL-only symbol '%s'\n", m, e, s); break; case export_unused_gpl: - fatal("modpost: GPL-incompatible module %s%s " + fatal("GPL-incompatible module %s%s " "uses GPL-only symbol marked UNUSED '%s'\n", m, e, s); break; case export_gpl_future: - warn("modpost: GPL-incompatible module %s%s " + warn("GPL-incompatible module %s%s " "uses future GPL-only symbol '%s'\n", m, e, s); break; case export_plain: @@ -2178,7 +2176,7 @@ static void check_for_unused(enum export exp, const char *m, const char *s) switch (exp) { case export_unused: case export_unused_gpl: - warn("modpost: module %s%s " + warn("module %s%s " "uses symbol '%s' marked UNUSED\n", m, e, s); break; default: @@ -2197,14 +2195,11 @@ static int check_exports(struct module *mod) exp = find_symbol(s->name); if (!exp || exp->module == mod) { if (have_vmlinux && !s->weak) { - if (warn_unresolved) { - warn("\"%s\" [%s.ko] undefined!\n", - s->name, mod->name); - } else { - merror("\"%s\" [%s.ko] undefined!\n", - s->name, mod->name); + warn_unless(!warn_unresolved, + "\"%s\" [%s.ko] undefined!\n", + s->name, mod->name); + if (!warn_unresolved) err = 1; - } } continue; } @@ -2653,7 +2648,7 @@ int main(int argc, char **argv) if (dump_write) write_dump(dump_write); if (sec_mismatch_count && sec_mismatch_fatal) - fatal("modpost: Section mismatches detected.\n" + fatal("Section mismatches detected.\n" "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n"); for (n = 0; n < SYMBOL_HASH_SIZE; n++) { struct symbol *s; diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index 64a82d2d85f6..631d07714f7a 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -198,6 +198,22 @@ void *grab_file(const char *filename, unsigned long *size); char* get_next_line(unsigned long *pos, void *file, unsigned long size); void release_file(void *file, unsigned long size); -void fatal(const char *fmt, ...); -void warn(const char *fmt, ...); -void merror(const char *fmt, ...); +enum loglevel { + LOG_WARN, + LOG_ERROR, + LOG_FATAL +}; + +void modpost_log(enum loglevel loglevel, const char *fmt, ...); + +#define warn(fmt, args...) modpost_log(LOG_WARN, fmt, ##args) +#define merror(fmt, args...) modpost_log(LOG_ERROR, fmt, ##args) +#define fatal(fmt, args...) modpost_log(LOG_FATAL, fmt, ##args) +/* Warn unless condition is true, then use merror() */ +#define warn_unless(condition, fmt, args...) \ +do { \ + if (condition) \ + merror(fmt, ##args); \ + else \ + warn(fmt, ##args); \ +} while (0) -- 2.16.4
| |