modprobe: add --show-exports
diff mbox series

Message ID 20181108115637.27597-1-yauheni.kaliuta@redhat.com
State New
Headers show
Series
  • modprobe: add --show-exports
Related show

Commit Message

Yauheni Kaliuta Nov. 8, 2018, 11:56 a.m. UTC
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(-)

Comments

Yauheni Kaliuta Nov. 8, 2018, 12:28 p.m. UTC | #1
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
Lucas De Marchi Nov. 12, 2018, 11:01 p.m. UTC | #2
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

Patch
diff mbox series

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)