| Message ID | 20220424190811.1678416-7-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: yet another series of cleanups (modpost and LTO) | expand |
/On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Use 'bool' to clarify that the valid value is true or false. > > Here is a small note for the conversion. > > Strictly speaking, module::gpl_compatible was not boolean because > new_module() initialized it to -1. Maybe, -1 was used to represent the > license is 'unknown', but it is not useful. > > Since commit 1d6cd3929360 ("modpost: turn missing MODULE_LICENSE() into > error"), unknown module license is not allowed anyway. > > I changed the initializer "= -1" to "= true". This has no functional > change. > > The current code: > > if (!mod->gpl_compatible) > check_for_gpl_usage(exp->export, basename, exp->name); > > ... only checks whether gpl_compabilt is zero or not: s/gpl_compabilt/gpl_compatible/ Also the trailing `:` should perhaps be `.`. Shouldn't gpl_compatible default to false, until proven otherwise? What happens if you default to false? Perhaps an identifier like `maybe_gpl_compatible` would be more descriptive? Also, if we're going to rename a few vars, consider using prefixes like is_*, has_*, or should_* for some of these to improve the readability for boolean variables. Otherwise LGTM. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > scripts/mod/modpost.c | 60 +++++++++++++++++++++---------------------- > scripts/mod/modpost.h | 10 ++++---- > 2 files changed, 35 insertions(+), 35 deletions(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index f9cbb6b6b7a5..52dd07a36379 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -23,20 +23,20 @@ > #include "../../include/linux/license.h" > > /* Are we using CONFIG_MODVERSIONS? */ > -static int modversions; > +static bool modversions; > /* Is CONFIG_MODULE_SRCVERSION_ALL set? */ > -static int all_versions; > +static bool all_versions; > /* If we are modposting external module set to 1 */ > -static int external_module; > +static bool external_module; > /* Only warn about unresolved symbols */ > -static int warn_unresolved; > +static bool warn_unresolved; > /* How a symbol is exported */ > static int sec_mismatch_count; > -static int sec_mismatch_warn_only = true; > +static bool sec_mismatch_warn_only = true; > /* ignore missing files */ > -static int ignore_missing_files; > +static bool ignore_missing_files; > /* If set to 1, only warn (instead of error) about missing ns imports */ > -static int allow_missing_ns_imports; > +static bool allow_missing_ns_imports; > > static bool error_occurred; > > @@ -187,7 +187,7 @@ static struct module *new_module(const char *modname) > /* add to list */ > strcpy(mod->name, modname); > mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0); > - mod->gpl_compatible = -1; > + mod->gpl_compatible = true; > mod->next = modules; > modules = mod; > > @@ -203,10 +203,10 @@ struct symbol { > struct symbol *next; > struct module *module; > unsigned int crc; > - int crc_valid; > + bool crc_valid; > char *namespace; > - unsigned int weak:1; > - unsigned int is_static:1; /* 1 if symbol is not global */ > + bool weak; > + bool is_static; /* true if symbol is not global */ > enum export export; /* Type of export */ > char name[]; > }; > @@ -230,7 +230,7 @@ static inline unsigned int tdb_hash(const char *name) > * Allocate a new symbols for use in the hash of exported symbols or > * the list of unresolved symbols per module > **/ > -static struct symbol *alloc_symbol(const char *name, unsigned int weak, > +static struct symbol *alloc_symbol(const char *name, bool weak, > struct symbol *next) > { > struct symbol *s = NOFAIL(malloc(sizeof(*s) + strlen(name) + 1)); > @@ -239,7 +239,7 @@ static struct symbol *alloc_symbol(const char *name, unsigned int weak, > strcpy(s->name, name); > s->weak = weak; > s->next = next; > - s->is_static = 1; > + s->is_static = true; > return s; > } > > @@ -250,7 +250,7 @@ static struct symbol *new_symbol(const char *name, struct module *module, > unsigned int hash; > > hash = tdb_hash(name) % SYMBOL_HASH_SIZE; > - symbolhash[hash] = alloc_symbol(name, 0, symbolhash[hash]); > + symbolhash[hash] = alloc_symbol(name, false, symbolhash[hash]); > > return symbolhash[hash]; > } > @@ -419,7 +419,7 @@ static void sym_set_crc(const char *name, unsigned int crc) > return; > > s->crc = crc; > - s->crc_valid = 1; > + s->crc_valid = true; > } > > static void *grab_file(const char *filename, size_t *size) > @@ -716,9 +716,9 @@ static void handle_symbol(struct module *mod, struct elf_info *info, > sym_add_exported(name, mod, export); > } > if (strcmp(symname, "init_module") == 0) > - mod->has_init = 1; > + mod->has_init = true; > if (strcmp(symname, "cleanup_module") == 0) > - mod->has_cleanup = 1; > + mod->has_cleanup = true; > break; > } > } > @@ -2008,9 +2008,9 @@ static void read_symbols(const char *modname) > error("missing MODULE_LICENSE() in %s\n", modname); > while (license) { > if (license_is_gpl_compatible(license)) > - mod->gpl_compatible = 1; > + mod->gpl_compatible = true; > else { > - mod->gpl_compatible = 0; > + mod->gpl_compatible = false; > break; > } > license = get_next_modinfo(&info, "license", license); > @@ -2053,7 +2053,7 @@ static void read_symbols(const char *modname) > sym->st_name)); > > if (s) > - s->is_static = 0; > + s->is_static = false; > } > } > > @@ -2073,7 +2073,7 @@ static void read_symbols(const char *modname) > * the automatic versioning doesn't pick it up, but it's really > * important anyhow */ > if (modversions) > - mod->unres = alloc_symbol("module_layout", 0, mod->unres); > + mod->unres = alloc_symbol("module_layout", false, mod->unres); > } > > static void read_symbols_from_files(const char *filename) > @@ -2305,7 +2305,7 @@ static void add_depends(struct buffer *b, struct module *mod) > if (s->module->seen) > continue; > > - s->module->seen = 1; > + s->module->seen = true; > p = strrchr(s->module->name, '/'); > if (p) > p++; > @@ -2422,10 +2422,10 @@ static void read_dump(const char *fname) > mod = find_module(modname); > if (!mod) { > mod = new_module(modname); > - mod->from_dump = 1; > + mod->from_dump = true; > } > s = sym_add_exported(symname, mod, export_no(export)); > - s->is_static = 0; > + s->is_static = false; > sym_set_crc(symname, crc); > sym_update_namespace(symname, namespace); > } > @@ -2503,7 +2503,7 @@ int main(int argc, char **argv) > while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) { > switch (opt) { > case 'e': > - external_module = 1; > + external_module = true; > break; > case 'i': > *dump_read_iter = > @@ -2512,28 +2512,28 @@ int main(int argc, char **argv) > dump_read_iter = &(*dump_read_iter)->next; > break; > case 'm': > - modversions = 1; > + modversions = true; > break; > case 'n': > - ignore_missing_files = 1; > + ignore_missing_files = true; > break; > case 'o': > dump_write = optarg; > break; > case 'a': > - all_versions = 1; > + all_versions = true; > break; > case 'T': > files_source = optarg; > break; > case 'w': > - warn_unresolved = 1; > + warn_unresolved = true; > break; > case 'E': > sec_mismatch_warn_only = false; > break; > case 'N': > - allow_missing_ns_imports = 1; > + allow_missing_ns_imports = true; > break; > case 'd': > missing_namespace_deps = optarg; > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h > index a85dcec3669a..4085bf5b33aa 100644 > --- a/scripts/mod/modpost.h > +++ b/scripts/mod/modpost.h > @@ -1,4 +1,5 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > +#include <stdbool.h> > #include <stdio.h> > #include <stdlib.h> > #include <stdarg.h> > @@ -116,11 +117,10 @@ struct module { > struct module *next; > int gpl_compatible; > struct symbol *unres; > - int from_dump; /* 1 if module was loaded from *.symvers */ > - int is_vmlinux; > - int seen; > - int has_init; > - int has_cleanup; > + bool from_dump; /* true if module was loaded from *.symvers */ > + bool is_vmlinux; > + bool seen; > + bool has_init, has_cleanup; > struct buffer dev_table_buf; > char srcversion[25]; > // Missing namespace dependencies > -- > 2.32.0 >
On Mon, Apr 25, 2022 at 11:34 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > /On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > scripts/mod/modpost.c | 60 +++++++++++++++++++++---------------------- > > scripts/mod/modpost.h | 10 ++++---- > > 2 files changed, 35 insertions(+), 35 deletions(-) > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > index f9cbb6b6b7a5..52dd07a36379 100644 > > --- a/scripts/mod/modpost.c > > +++ b/scripts/mod/modpost.c > > @@ -203,10 +203,10 @@ struct symbol { > > struct symbol *next; > > struct module *module; > > unsigned int crc; > > - int crc_valid; > > + bool crc_valid; > > char *namespace; > > - unsigned int weak:1; > > - unsigned int is_static:1; /* 1 if symbol is not global */ > > + bool weak; > > + bool is_static; /* true if symbol is not global */ > > enum export export; /* Type of export */ > > char name[]; > > }; This will change the sizeof(struct symbol). I'm guessing we have lots of symbols to process? If we have many live at once, perhaps it would be better to keep these as bitfields, but additionally move them to the end of the struct definition so as to save space?
On Tue, Apr 26, 2022 at 3:56 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Mon, Apr 25, 2022 at 11:34 AM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > /On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > --- > > > > > > scripts/mod/modpost.c | 60 +++++++++++++++++++++---------------------- > > > scripts/mod/modpost.h | 10 ++++---- > > > 2 files changed, 35 insertions(+), 35 deletions(-) > > > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > > index f9cbb6b6b7a5..52dd07a36379 100644 > > > --- a/scripts/mod/modpost.c > > > +++ b/scripts/mod/modpost.c > > > @@ -203,10 +203,10 @@ struct symbol { > > > struct symbol *next; > > > struct module *module; > > > unsigned int crc; > > > - int crc_valid; > > > + bool crc_valid; > > > char *namespace; > > > - unsigned int weak:1; > > > - unsigned int is_static:1; /* 1 if symbol is not global */ > > > + bool weak; > > > + bool is_static; /* true if symbol is not global */ > > > enum export export; /* Type of export */ > > > char name[]; > > > }; > > This will change the sizeof(struct symbol). I'm guessing we have lots > of symbols to process? If we have many live at once, perhaps it would > be better to keep these as bitfields, but additionally move them to > the end of the struct definition so as to save space? Not really. On 64 bit systems, sizeof(struct symbol) is still 40 bytes. We may save some space by using bit fields and pahole analysis, but I do not think that is a big deal for userspace programs. I tend to prioritize code readability.
On Tue, Apr 26, 2022 at 3:34 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > /On Sun, Apr 24, 2022 at 12:09 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > Use 'bool' to clarify that the valid value is true or false. > > > > Here is a small note for the conversion. > > > > Strictly speaking, module::gpl_compatible was not boolean because > > new_module() initialized it to -1. Maybe, -1 was used to represent the > > license is 'unknown', but it is not useful. > > > > Since commit 1d6cd3929360 ("modpost: turn missing MODULE_LICENSE() into > > error"), unknown module license is not allowed anyway. > > > > I changed the initializer "= -1" to "= true". This has no functional > > change. > > > > The current code: > > > > if (!mod->gpl_compatible) > > check_for_gpl_usage(exp->export, basename, exp->name); > > > > ... only checks whether gpl_compabilt is zero or not: > > s/gpl_compabilt/gpl_compatible/ > > Also the trailing `:` should perhaps be `.`. > > Shouldn't gpl_compatible default to false, until proven otherwise? > What happens if you default to false? Perhaps an identifier like > `maybe_gpl_compatible` would be more descriptive? The init value should be 'true'. In v2, I split this change into a separate patch, and added some comments. https://patchwork.kernel.org/project/linux-kbuild/patch/20220501084032.1025918-3-masahiroy@kernel.org/ I hope it answered your question. > Also, if we're going to rename a few vars, consider using prefixes > like is_*, has_*, or should_* for some of these to improve the > readability for boolean variables. OK, I renamed it to is_gpl_compatible in v2. > Otherwise LGTM. > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > scripts/mod/modpost.c | 60 +++++++++++++++++++++---------------------- > > scripts/mod/modpost.h | 10 ++++---- > > 2 files changed, 35 insertions(+), 35 deletions(-) > > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > > index f9cbb6b6b7a5..52dd07a36379 100644 > > --- a/scripts/mod/modpost.c > > +++ b/scripts/mod/modpost.c > > @@ -23,20 +23,20 @@ > > #include "../../include/linux/license.h" > > > > /* Are we using CONFIG_MODVERSIONS? */ > > -static int modversions; > > +static bool modversions; > > /* Is CONFIG_MODULE_SRCVERSION_ALL set? */ > > -static int all_versions; > > +static bool all_versions; > > /* If we are modposting external module set to 1 */ > > -static int external_module; > > +static bool external_module; > > /* Only warn about unresolved symbols */ > > -static int warn_unresolved; > > +static bool warn_unresolved; > > /* How a symbol is exported */ > > static int sec_mismatch_count; > > -static int sec_mismatch_warn_only = true; > > +static bool sec_mismatch_warn_only = true; > > /* ignore missing files */ > > -static int ignore_missing_files; > > +static bool ignore_missing_files; > > /* If set to 1, only warn (instead of error) about missing ns imports */ > > -static int allow_missing_ns_imports; > > +static bool allow_missing_ns_imports; > > > > static bool error_occurred; > > > > @@ -187,7 +187,7 @@ static struct module *new_module(const char *modname) > > /* add to list */ > > strcpy(mod->name, modname); > > mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0); > > - mod->gpl_compatible = -1; > > + mod->gpl_compatible = true; > > mod->next = modules; > > modules = mod; > > > > @@ -203,10 +203,10 @@ struct symbol { > > struct symbol *next; > > struct module *module; > > unsigned int crc; > > - int crc_valid; > > + bool crc_valid; > > char *namespace; > > - unsigned int weak:1; > > - unsigned int is_static:1; /* 1 if symbol is not global */ > > + bool weak; > > + bool is_static; /* true if symbol is not global */ > > enum export export; /* Type of export */ > > char name[]; > > }; > > @@ -230,7 +230,7 @@ static inline unsigned int tdb_hash(const char *name) > > * Allocate a new symbols for use in the hash of exported symbols or > > * the list of unresolved symbols per module > > **/ > > -static struct symbol *alloc_symbol(const char *name, unsigned int weak, > > +static struct symbol *alloc_symbol(const char *name, bool weak, > > struct symbol *next) > > { > > struct symbol *s = NOFAIL(malloc(sizeof(*s) + strlen(name) + 1)); > > @@ -239,7 +239,7 @@ static struct symbol *alloc_symbol(const char *name, unsigned int weak, > > strcpy(s->name, name); > > s->weak = weak; > > s->next = next; > > - s->is_static = 1; > > + s->is_static = true; > > return s; > > } > > > > @@ -250,7 +250,7 @@ static struct symbol *new_symbol(const char *name, struct module *module, > > unsigned int hash; > > > > hash = tdb_hash(name) % SYMBOL_HASH_SIZE; > > - symbolhash[hash] = alloc_symbol(name, 0, symbolhash[hash]); > > + symbolhash[hash] = alloc_symbol(name, false, symbolhash[hash]); > > > > return symbolhash[hash]; > > } > > @@ -419,7 +419,7 @@ static void sym_set_crc(const char *name, unsigned int crc) > > return; > > > > s->crc = crc; > > - s->crc_valid = 1; > > + s->crc_valid = true; > > } > > > > static void *grab_file(const char *filename, size_t *size) > > @@ -716,9 +716,9 @@ static void handle_symbol(struct module *mod, struct elf_info *info, > > sym_add_exported(name, mod, export); > > } > > if (strcmp(symname, "init_module") == 0) > > - mod->has_init = 1; > > + mod->has_init = true; > > if (strcmp(symname, "cleanup_module") == 0) > > - mod->has_cleanup = 1; > > + mod->has_cleanup = true; > > break; > > } > > } > > @@ -2008,9 +2008,9 @@ static void read_symbols(const char *modname) > > error("missing MODULE_LICENSE() in %s\n", modname); > > while (license) { > > if (license_is_gpl_compatible(license)) > > - mod->gpl_compatible = 1; > > + mod->gpl_compatible = true; > > else { > > - mod->gpl_compatible = 0; > > + mod->gpl_compatible = false; > > break; > > } > > license = get_next_modinfo(&info, "license", license); > > @@ -2053,7 +2053,7 @@ static void read_symbols(const char *modname) > > sym->st_name)); > > > > if (s) > > - s->is_static = 0; > > + s->is_static = false; > > } > > } > > > > @@ -2073,7 +2073,7 @@ static void read_symbols(const char *modname) > > * the automatic versioning doesn't pick it up, but it's really > > * important anyhow */ > > if (modversions) > > - mod->unres = alloc_symbol("module_layout", 0, mod->unres); > > + mod->unres = alloc_symbol("module_layout", false, mod->unres); > > } > > > > static void read_symbols_from_files(const char *filename) > > @@ -2305,7 +2305,7 @@ static void add_depends(struct buffer *b, struct module *mod) > > if (s->module->seen) > > continue; > > > > - s->module->seen = 1; > > + s->module->seen = true; > > p = strrchr(s->module->name, '/'); > > if (p) > > p++; > > @@ -2422,10 +2422,10 @@ static void read_dump(const char *fname) > > mod = find_module(modname); > > if (!mod) { > > mod = new_module(modname); > > - mod->from_dump = 1; > > + mod->from_dump = true; > > } > > s = sym_add_exported(symname, mod, export_no(export)); > > - s->is_static = 0; > > + s->is_static = false; > > sym_set_crc(symname, crc); > > sym_update_namespace(symname, namespace); > > } > > @@ -2503,7 +2503,7 @@ int main(int argc, char **argv) > > while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) { > > switch (opt) { > > case 'e': > > - external_module = 1; > > + external_module = true; > > break; > > case 'i': > > *dump_read_iter = > > @@ -2512,28 +2512,28 @@ int main(int argc, char **argv) > > dump_read_iter = &(*dump_read_iter)->next; > > break; > > case 'm': > > - modversions = 1; > > + modversions = true; > > break; > > case 'n': > > - ignore_missing_files = 1; > > + ignore_missing_files = true; > > break; > > case 'o': > > dump_write = optarg; > > break; > > case 'a': > > - all_versions = 1; > > + all_versions = true; > > break; > > case 'T': > > files_source = optarg; > > break; > > case 'w': > > - warn_unresolved = 1; > > + warn_unresolved = true; > > break; > > case 'E': > > sec_mismatch_warn_only = false; > > break; > > case 'N': > > - allow_missing_ns_imports = 1; > > + allow_missing_ns_imports = true; > > break; > > case 'd': > > missing_namespace_deps = optarg; > > diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h > > index a85dcec3669a..4085bf5b33aa 100644 > > --- a/scripts/mod/modpost.h > > +++ b/scripts/mod/modpost.h > > @@ -1,4 +1,5 @@ > > /* SPDX-License-Identifier: GPL-2.0 */ > > +#include <stdbool.h> > > #include <stdio.h> > > #include <stdlib.h> > > #include <stdarg.h> > > @@ -116,11 +117,10 @@ struct module { > > struct module *next; > > int gpl_compatible; > > struct symbol *unres; > > - int from_dump; /* 1 if module was loaded from *.symvers */ > > - int is_vmlinux; > > - int seen; > > - int has_init; > > - int has_cleanup; > > + bool from_dump; /* true if module was loaded from *.symvers */ > > + bool is_vmlinux; > > + bool seen; > > + bool has_init, has_cleanup; > > struct buffer dev_table_buf; > > char srcversion[25]; > > // Missing namespace dependencies > > -- > > 2.32.0 > > > > > -- > Thanks, > ~Nick Desaulniers
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index f9cbb6b6b7a5..52dd07a36379 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -23,20 +23,20 @@ #include "../../include/linux/license.h" /* Are we using CONFIG_MODVERSIONS? */ -static int modversions; +static bool modversions; /* Is CONFIG_MODULE_SRCVERSION_ALL set? */ -static int all_versions; +static bool all_versions; /* If we are modposting external module set to 1 */ -static int external_module; +static bool external_module; /* Only warn about unresolved symbols */ -static int warn_unresolved; +static bool warn_unresolved; /* How a symbol is exported */ static int sec_mismatch_count; -static int sec_mismatch_warn_only = true; +static bool sec_mismatch_warn_only = true; /* ignore missing files */ -static int ignore_missing_files; +static bool ignore_missing_files; /* If set to 1, only warn (instead of error) about missing ns imports */ -static int allow_missing_ns_imports; +static bool allow_missing_ns_imports; static bool error_occurred; @@ -187,7 +187,7 @@ static struct module *new_module(const char *modname) /* add to list */ strcpy(mod->name, modname); mod->is_vmlinux = (strcmp(modname, "vmlinux") == 0); - mod->gpl_compatible = -1; + mod->gpl_compatible = true; mod->next = modules; modules = mod; @@ -203,10 +203,10 @@ struct symbol { struct symbol *next; struct module *module; unsigned int crc; - int crc_valid; + bool crc_valid; char *namespace; - unsigned int weak:1; - unsigned int is_static:1; /* 1 if symbol is not global */ + bool weak; + bool is_static; /* true if symbol is not global */ enum export export; /* Type of export */ char name[]; }; @@ -230,7 +230,7 @@ static inline unsigned int tdb_hash(const char *name) * Allocate a new symbols for use in the hash of exported symbols or * the list of unresolved symbols per module **/ -static struct symbol *alloc_symbol(const char *name, unsigned int weak, +static struct symbol *alloc_symbol(const char *name, bool weak, struct symbol *next) { struct symbol *s = NOFAIL(malloc(sizeof(*s) + strlen(name) + 1)); @@ -239,7 +239,7 @@ static struct symbol *alloc_symbol(const char *name, unsigned int weak, strcpy(s->name, name); s->weak = weak; s->next = next; - s->is_static = 1; + s->is_static = true; return s; } @@ -250,7 +250,7 @@ static struct symbol *new_symbol(const char *name, struct module *module, unsigned int hash; hash = tdb_hash(name) % SYMBOL_HASH_SIZE; - symbolhash[hash] = alloc_symbol(name, 0, symbolhash[hash]); + symbolhash[hash] = alloc_symbol(name, false, symbolhash[hash]); return symbolhash[hash]; } @@ -419,7 +419,7 @@ static void sym_set_crc(const char *name, unsigned int crc) return; s->crc = crc; - s->crc_valid = 1; + s->crc_valid = true; } static void *grab_file(const char *filename, size_t *size) @@ -716,9 +716,9 @@ static void handle_symbol(struct module *mod, struct elf_info *info, sym_add_exported(name, mod, export); } if (strcmp(symname, "init_module") == 0) - mod->has_init = 1; + mod->has_init = true; if (strcmp(symname, "cleanup_module") == 0) - mod->has_cleanup = 1; + mod->has_cleanup = true; break; } } @@ -2008,9 +2008,9 @@ static void read_symbols(const char *modname) error("missing MODULE_LICENSE() in %s\n", modname); while (license) { if (license_is_gpl_compatible(license)) - mod->gpl_compatible = 1; + mod->gpl_compatible = true; else { - mod->gpl_compatible = 0; + mod->gpl_compatible = false; break; } license = get_next_modinfo(&info, "license", license); @@ -2053,7 +2053,7 @@ static void read_symbols(const char *modname) sym->st_name)); if (s) - s->is_static = 0; + s->is_static = false; } } @@ -2073,7 +2073,7 @@ static void read_symbols(const char *modname) * the automatic versioning doesn't pick it up, but it's really * important anyhow */ if (modversions) - mod->unres = alloc_symbol("module_layout", 0, mod->unres); + mod->unres = alloc_symbol("module_layout", false, mod->unres); } static void read_symbols_from_files(const char *filename) @@ -2305,7 +2305,7 @@ static void add_depends(struct buffer *b, struct module *mod) if (s->module->seen) continue; - s->module->seen = 1; + s->module->seen = true; p = strrchr(s->module->name, '/'); if (p) p++; @@ -2422,10 +2422,10 @@ static void read_dump(const char *fname) mod = find_module(modname); if (!mod) { mod = new_module(modname); - mod->from_dump = 1; + mod->from_dump = true; } s = sym_add_exported(symname, mod, export_no(export)); - s->is_static = 0; + s->is_static = false; sym_set_crc(symname, crc); sym_update_namespace(symname, namespace); } @@ -2503,7 +2503,7 @@ int main(int argc, char **argv) while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) { switch (opt) { case 'e': - external_module = 1; + external_module = true; break; case 'i': *dump_read_iter = @@ -2512,28 +2512,28 @@ int main(int argc, char **argv) dump_read_iter = &(*dump_read_iter)->next; break; case 'm': - modversions = 1; + modversions = true; break; case 'n': - ignore_missing_files = 1; + ignore_missing_files = true; break; case 'o': dump_write = optarg; break; case 'a': - all_versions = 1; + all_versions = true; break; case 'T': files_source = optarg; break; case 'w': - warn_unresolved = 1; + warn_unresolved = true; break; case 'E': sec_mismatch_warn_only = false; break; case 'N': - allow_missing_ns_imports = 1; + allow_missing_ns_imports = true; break; case 'd': missing_namespace_deps = optarg; diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index a85dcec3669a..4085bf5b33aa 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -1,4 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0 */ +#include <stdbool.h> #include <stdio.h> #include <stdlib.h> #include <stdarg.h> @@ -116,11 +117,10 @@ struct module { struct module *next; int gpl_compatible; struct symbol *unres; - int from_dump; /* 1 if module was loaded from *.symvers */ - int is_vmlinux; - int seen; - int has_init; - int has_cleanup; + bool from_dump; /* true if module was loaded from *.symvers */ + bool is_vmlinux; + bool seen; + bool has_init, has_cleanup; struct buffer dev_table_buf; char srcversion[25]; // Missing namespace dependencies
Use 'bool' to clarify that the valid value is true or false. Here is a small note for the conversion. Strictly speaking, module::gpl_compatible was not boolean because new_module() initialized it to -1. Maybe, -1 was used to represent the license is 'unknown', but it is not useful. Since commit 1d6cd3929360 ("modpost: turn missing MODULE_LICENSE() into error"), unknown module license is not allowed anyway. I changed the initializer "= -1" to "= true". This has no functional change. The current code: if (!mod->gpl_compatible) check_for_gpl_usage(exp->export, basename, exp->name); ... only checks whether gpl_compabilt is zero or not: Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/modpost.c | 60 +++++++++++++++++++++---------------------- scripts/mod/modpost.h | 10 ++++---- 2 files changed, 35 insertions(+), 35 deletions(-)