Re: [PATCH] modprobe: add --show-exports

[Lucas De Marchi <lucas.de.marchi@gmail.com>]

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



-- 
Lucas De Marchi