diff mbox series

modinfo: dont print module name when --field is given

Message ID 20200823215433.j5gc5rnsmahpf43v@blumerang (mailing list archive)
State New, archived
Headers show
Series modinfo: dont print module name when --field is given | expand

Commit Message

Xaver Hörl Aug. 23, 2020, 9:54 p.m. UTC
I hope I found the right mailing list for this issue, which
came up here: https://github.com/NixOS/nixpkgs/pull/96008

The man page for modinfo claims that with the -F / --field option only
the specified field gets printed. Currently this is not true as for builtin modules, the name is always printed.

Try something like "modinfo -F firmware unix" for example.
It will print: "name:           unix"

So either the man page needs to be updated or the behaviour changed.
The following patch should fix this:

---
 tools/modinfo.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--
2.28.0

Comments

Lucas De Marchi Aug. 25, 2020, 5:27 p.m. UTC | #1
On Sun, Aug 23, 2020 at 3:01 PM Xaver Hörl <hoe.dom@gmx.de> wrote:
>
> I hope I found the right mailing list for this issue, which
> came up here: https://github.com/NixOS/nixpkgs/pull/96008
>
> The man page for modinfo claims that with the -F / --field option only
> the specified field gets printed. Currently this is not true as for builtin modules, the name is always printed.
>
> Try something like "modinfo -F firmware unix" for example.
> It will print: "name:           unix"
>
> So either the man page needs to be updated or the behaviour changed.

yeah, it seems something to change in the code. This behavior
started when we added support in modinfo to show information
about builtin modules.

> The following patch should fix this:
>
> ---
>  tools/modinfo.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/modinfo.c b/tools/modinfo.c
> index 0231bb0..7b2259e 100644
> --- a/tools/modinfo.c
> +++ b/tools/modinfo.c
> @@ -178,7 +178,10 @@ static int modinfo_do(struct kmod_module *mod)
>         is_builtin = (filename == NULL);
>
>         if (is_builtin) {
> -               printf("%-16s%s%c", "name:", kmod_module_get_name(mod), separator);
> +               if (field == NULL || field != NULL && streq(field, "name")){

the name would be handled by the next if/else branch. Unconditionally printing
the name at the beginning for built-in modules doesn't seem something important
to retain. I'd rather handle it in the `if (is_builtin && err ==
-ENOENT) {` below just
not to exit without printing anything. In that case, printing both
name and filename
would be preferred, so the user knows why there isn't additional information

thanks
Lucas De Marchi

> +                       printf("%-16s%s%c", "name:",
> +                              kmod_module_get_name(mod), separator);
> +               }
>                 filename = "(builtin)";
>         }
>
> --
> 2.28.0
>
diff mbox series

Patch

diff --git a/tools/modinfo.c b/tools/modinfo.c
index 0231bb0..7b2259e 100644
--- a/tools/modinfo.c
+++ b/tools/modinfo.c
@@ -178,7 +178,10 @@  static int modinfo_do(struct kmod_module *mod)
 	is_builtin = (filename == NULL);

 	if (is_builtin) {
-		printf("%-16s%s%c", "name:", kmod_module_get_name(mod), separator);
+		if (field == NULL || field != NULL && streq(field, "name")){
+			printf("%-16s%s%c", "name:",
+			       kmod_module_get_name(mod), separator);
+		}
 		filename = "(builtin)";
 	}