On Thu, Nov 8, 2018 at 4:29 AM Yauheni Kaliuta
[off-list ref] 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?

quoted

quoted

quoted

quoted

quoted

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 [off-list ref]
 > ---
 >  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