| Message ID | 20181108115637.27597-1-yauheni.kaliuta@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | modprobe: add --show-exports | expand |
Hi! Actually it doesn't sound good because of the API: [...] kmod_module_versions_free_list(list); kmod_module_unref(mod); It is supposed that the list produced by kmod_module_get_versions() only, not kmod_module_get_symbols(), even if the structures actually have the same fields. Would it be good if I change the internals a bit, to use only struct kmod_module_symbol (it is possible to keep external API the same for compatibility)? Or there are reasons to keep them different now? >>>>> On Thu, 8 Nov 2018 13:56:37 +0200, Yauheni Kaliuta wrote: > modprobe has --show-modversions switch, which dumps symbols with > their modversion crcs from the __versions sections. > At the moment the section contains information for the dependency > symbols only, while exported symbols add to symtab entries with > __crc_ prefix (the format may differ, see 1e48901166ef libkmod-elf: > resolve CRC if module is built with MODULE_REL_CRCS). > The patch makes it to show exported symbols as well. > It refactors the --show-modversions code to avoid duplications. > Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com> > --- > tools/modprobe.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > diff --git a/tools/modprobe.c b/tools/modprobe.c > index 43605ccaf0f0..97da4e6986ce 100644 > --- a/tools/modprobe.c > +++ b/tools/modprobe.c > @@ -76,6 +76,8 @@ static const struct option cmdopts[] = { > {"show-config", no_argument, 0, 'c'}, > {"show-modversions", no_argument, 0, 4}, > {"dump-modversions", no_argument, 0, 4}, > + {"show-exports", no_argument, 0, 6}, > + {"dump-exports", no_argument, 0, 6}, > {"dry-run", no_argument, 0, 'n'}, > {"show", no_argument, 0, 'n'}, > @@ -124,6 +126,8 @@ static void help(void) > "\t-c, --show-config Same as --showconfig\n" > "\t --show-modversions Dump module symbol version and exit\n" > "\t --dump-modversions Same as --show-modversions\n" > + "\t --show-exports Dump module exported symbol versions and exit\n" > + "\t --dump-exports Same as --show-exports\n" > "\n" > "General Options:\n" > "\t-n, --dry-run Do not execute operations, just print out\n" > @@ -204,7 +208,9 @@ static int show_config(struct kmod_ctx *ctx) > return 0; > } > -static int show_modversions(struct kmod_ctx *ctx, const char *filename) > +static int show_versions(struct kmod_ctx *ctx, > + int (*get_versions_f)(const struct kmod_module *, struct kmod_list **), > + const char *filename) > { > struct kmod_list *l, *list = NULL; > struct kmod_module *mod; > @@ -214,7 +220,7 @@ static int show_modversions(struct kmod_ctx *ctx, const char *filename) > return err; > } > - err = kmod_module_get_versions(mod, &list); > + err = get_versions_f(mod, &list); > if (err < 0) { > LOG("could not get modversions of %s: %s\n", > filename, strerror(-err)); > @@ -232,6 +238,16 @@ static int show_modversions(struct kmod_ctx *ctx, const char *filename) > return 0; > } > +static int show_modversions(struct kmod_ctx *ctx, const char *filename) > +{ > + return show_versions(ctx, kmod_module_get_versions, filename); > +} > + > +static int show_exports(struct kmod_ctx *ctx, const char *filename) > +{ > + return show_versions(ctx, kmod_module_get_symbols, filename); > +} > + > static int command_do(struct kmod_module *module, const char *type, > const char *command, const char *cmdline_opts) > { > @@ -727,6 +743,7 @@ static int do_modprobe(int argc, char **orig_argv) > int do_remove = 0; > int do_show_config = 0; > int do_show_modversions = 0; > + int do_show_exports = 0; > int err; > argv = prepend_options_from_env(&argc, orig_argv); > @@ -783,6 +800,9 @@ static int do_modprobe(int argc, char **orig_argv) > case 4: > do_show_modversions = 1; > break; > + case 6: > + do_show_exports = 1; > + break; > case 'n': > dry_run = 1; > break; > @@ -886,6 +906,8 @@ static int do_modprobe(int argc, char **orig_argv) > err = show_config(ctx); > else if (do_show_modversions) > err = show_modversions(ctx, args[0]); > + else if (do_show_exports) > + err = show_exports(ctx, args[0]); > else if (do_remove) > err = rmmod_all(ctx, args, nargs); > else if (use_all) > -- > 2.19.1
On Thu, Nov 8, 2018 at 4:29 AM Yauheni Kaliuta <yauheni.kaliuta@redhat.com> wrote: > > Hi! > > Actually it doesn't sound good because of the API: > > [...] > > kmod_module_versions_free_list(list); > kmod_module_unref(mod); > > It is supposed that the list produced by > kmod_module_get_versions() only, not kmod_module_get_symbols(), > even if the structures actually have the same fields. > > Would it be good if I change the internals a bit, to use only > struct kmod_module_symbol (it is possible to keep external API > the same for compatibility)? it's in the TODO file: * review API, maybe unify all of these getters: - kmod_module_version_get_symbol() - kmod_module_version_get_crc() - kmod_module_symbol_get_symbol() - kmod_module_symbol_get_crc() - kmod_module_dependency_symbol_get_symbol() - kmod_module_dependency_symbol_get_crc() - kmod_module_versions_free_list() - kmod_module_symbols_free_list() - kmod_module_dependency_symbols_free_list() However this means breaking API, so it was never a great appeal. Maybe we could add new ones and keep the old functions for compatibility. Lucas De Marchi > > Or there are reasons to keep them different now? > > >>>>> On Thu, 8 Nov 2018 13:56:37 +0200, Yauheni Kaliuta wrote: > > > modprobe has --show-modversions switch, which dumps symbols with > > their modversion crcs from the __versions sections. > > > At the moment the section contains information for the dependency > > symbols only, while exported symbols add to symtab entries with > > __crc_ prefix (the format may differ, see 1e48901166ef libkmod-elf: > > resolve CRC if module is built with MODULE_REL_CRCS). > > > The patch makes it to show exported symbols as well. > > > It refactors the --show-modversions code to avoid duplications. > > > Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com> > > --- > > tools/modprobe.c | 26 ++++++++++++++++++++++++-- > > 1 file changed, 24 insertions(+), 2 deletions(-) > > > diff --git a/tools/modprobe.c b/tools/modprobe.c > > index 43605ccaf0f0..97da4e6986ce 100644 > > --- a/tools/modprobe.c > > +++ b/tools/modprobe.c > > @@ -76,6 +76,8 @@ static const struct option cmdopts[] = { > > {"show-config", no_argument, 0, 'c'}, > > {"show-modversions", no_argument, 0, 4}, > > {"dump-modversions", no_argument, 0, 4}, > > + {"show-exports", no_argument, 0, 6}, > > + {"dump-exports", no_argument, 0, 6}, > > > {"dry-run", no_argument, 0, 'n'}, > > {"show", no_argument, 0, 'n'}, > > @@ -124,6 +126,8 @@ static void help(void) > > "\t-c, --show-config Same as --showconfig\n" > > "\t --show-modversions Dump module symbol version and exit\n" > > "\t --dump-modversions Same as --show-modversions\n" > > + "\t --show-exports Dump module exported symbol versions and exit\n" > > + "\t --dump-exports Same as --show-exports\n" > > "\n" > > "General Options:\n" > > "\t-n, --dry-run Do not execute operations, just print out\n" > > @@ -204,7 +208,9 @@ static int show_config(struct kmod_ctx *ctx) > > return 0; > > } > > > -static int show_modversions(struct kmod_ctx *ctx, const char *filename) > > +static int show_versions(struct kmod_ctx *ctx, > > + int (*get_versions_f)(const struct kmod_module *, struct kmod_list **), > > + const char *filename) > > { > > struct kmod_list *l, *list = NULL; > > struct kmod_module *mod; > > @@ -214,7 +220,7 @@ static int show_modversions(struct kmod_ctx *ctx, const char *filename) > > return err; > > } > > > - err = kmod_module_get_versions(mod, &list); > > + err = get_versions_f(mod, &list); > > if (err < 0) { > > LOG("could not get modversions of %s: %s\n", > > filename, strerror(-err)); > > @@ -232,6 +238,16 @@ static int show_modversions(struct kmod_ctx *ctx, const char *filename) > > return 0; > > } > > > +static int show_modversions(struct kmod_ctx *ctx, const char *filename) > > +{ > > + return show_versions(ctx, kmod_module_get_versions, filename); > > +} > > + > > +static int show_exports(struct kmod_ctx *ctx, const char *filename) > > +{ > > + return show_versions(ctx, kmod_module_get_symbols, filename); > > +} > > + > > static int command_do(struct kmod_module *module, const char *type, > > const char *command, const char *cmdline_opts) > > { > > @@ -727,6 +743,7 @@ static int do_modprobe(int argc, char **orig_argv) > > int do_remove = 0; > > int do_show_config = 0; > > int do_show_modversions = 0; > > + int do_show_exports = 0; > > int err; > > > argv = prepend_options_from_env(&argc, orig_argv); > > @@ -783,6 +800,9 @@ static int do_modprobe(int argc, char **orig_argv) > > case 4: > > do_show_modversions = 1; > > break; > > + case 6: > > + do_show_exports = 1; > > + break; > > case 'n': > > dry_run = 1; > > break; > > @@ -886,6 +906,8 @@ static int do_modprobe(int argc, char **orig_argv) > > err = show_config(ctx); > > else if (do_show_modversions) > > err = show_modversions(ctx, args[0]); > > + else if (do_show_exports) > > + err = show_exports(ctx, args[0]); > > else if (do_remove) > > err = rmmod_all(ctx, args, nargs); > > else if (use_all) > > -- > > 2.19.1 > > > -- > WBR, > Yauheni Kaliuta
diff --git a/tools/modprobe.c b/tools/modprobe.c index 43605ccaf0f0..97da4e6986ce 100644 --- a/tools/modprobe.c +++ b/tools/modprobe.c @@ -76,6 +76,8 @@ static const struct option cmdopts[] = { {"show-config", no_argument, 0, 'c'}, {"show-modversions", no_argument, 0, 4}, {"dump-modversions", no_argument, 0, 4}, + {"show-exports", no_argument, 0, 6}, + {"dump-exports", no_argument, 0, 6}, {"dry-run", no_argument, 0, 'n'}, {"show", no_argument, 0, 'n'}, @@ -124,6 +126,8 @@ static void help(void) "\t-c, --show-config Same as --showconfig\n" "\t --show-modversions Dump module symbol version and exit\n" "\t --dump-modversions Same as --show-modversions\n" + "\t --show-exports Dump module exported symbol versions and exit\n" + "\t --dump-exports Same as --show-exports\n" "\n" "General Options:\n" "\t-n, --dry-run Do not execute operations, just print out\n" @@ -204,7 +208,9 @@ static int show_config(struct kmod_ctx *ctx) return 0; } -static int show_modversions(struct kmod_ctx *ctx, const char *filename) +static int show_versions(struct kmod_ctx *ctx, + int (*get_versions_f)(const struct kmod_module *, struct kmod_list **), + const char *filename) { struct kmod_list *l, *list = NULL; struct kmod_module *mod; @@ -214,7 +220,7 @@ static int show_modversions(struct kmod_ctx *ctx, const char *filename) return err; } - err = kmod_module_get_versions(mod, &list); + err = get_versions_f(mod, &list); if (err < 0) { LOG("could not get modversions of %s: %s\n", filename, strerror(-err)); @@ -232,6 +238,16 @@ static int show_modversions(struct kmod_ctx *ctx, const char *filename) return 0; } +static int show_modversions(struct kmod_ctx *ctx, const char *filename) +{ + return show_versions(ctx, kmod_module_get_versions, filename); +} + +static int show_exports(struct kmod_ctx *ctx, const char *filename) +{ + return show_versions(ctx, kmod_module_get_symbols, filename); +} + static int command_do(struct kmod_module *module, const char *type, const char *command, const char *cmdline_opts) { @@ -727,6 +743,7 @@ static int do_modprobe(int argc, char **orig_argv) int do_remove = 0; int do_show_config = 0; int do_show_modversions = 0; + int do_show_exports = 0; int err; argv = prepend_options_from_env(&argc, orig_argv); @@ -783,6 +800,9 @@ static int do_modprobe(int argc, char **orig_argv) case 4: do_show_modversions = 1; break; + case 6: + do_show_exports = 1; + break; case 'n': dry_run = 1; break; @@ -886,6 +906,8 @@ static int do_modprobe(int argc, char **orig_argv) err = show_config(ctx); else if (do_show_modversions) err = show_modversions(ctx, args[0]); + else if (do_show_exports) + err = show_exports(ctx, args[0]); else if (do_remove) err = rmmod_all(ctx, args, nargs); else if (use_all)
modprobe has --show-modversions switch, which dumps symbols with their modversion crcs from the __versions sections. At the moment the section contains information for the dependency symbols only, while exported symbols add to symtab entries with __crc_ prefix (the format may differ, see 1e48901166ef libkmod-elf: resolve CRC if module is built with MODULE_REL_CRCS). The patch makes it to show exported symbols as well. It refactors the --show-modversions code to avoid duplications. Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com> --- tools/modprobe.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)