* [PATCH] modinfo: dont print module name when --field is given @ 2020-08-23 21:54 Xaver Hörl 2020-08-25 17:27 ` Lucas De Marchi 0 siblings, 1 reply; 4+ messages in thread From: Xaver Hörl @ 2020-08-23 21:54 UTC (permalink / raw) To: linux-modules 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(-) 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)"; } -- 2.28.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] modinfo: dont print module name when --field is given 2020-08-23 21:54 [PATCH] modinfo: dont print module name when --field is given Xaver Hörl @ 2020-08-25 17:27 ` Lucas De Marchi 2020-08-26 9:36 ` Xaver Hörl 0 siblings, 1 reply; 4+ messages in thread From: Lucas De Marchi @ 2020-08-25 17:27 UTC (permalink / raw) To: Xaver Hörl; +Cc: linux-modules 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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] modinfo: dont print module name when --field is given 2020-08-25 17:27 ` Lucas De Marchi @ 2020-08-26 9:36 ` Xaver Hörl 2020-08-26 10:12 ` Lucas De Marchi 0 siblings, 1 reply; 4+ messages in thread From: Xaver Hörl @ 2020-08-26 9:36 UTC (permalink / raw) To: linux-modules; +Cc: Lucas De Marchi On Tue, Aug 25, 2020 at 10:27:46AM -0700, Lucas De Marchi wrote: > 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 Dropping the "name:" part with --field seems the right thing (updated patch below implements this). But I don't know what exactly to do about the error case, not sure if I get your intention right there. If --field is not given, the name and filename (i.e. "(builtin)" in this case) will be there anyway, on the other hand if --field _is_ given it seems a bit inconsistent to give additional output. Unless you consider this case an error condition of sorts, but the return value being 0 suggests otherwise. Thanks for having a look at this! Xaver Hörl --- tools/modinfo.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/modinfo.c b/tools/modinfo.c index 0231bb0..b49f622 100644 --- a/tools/modinfo.c +++ b/tools/modinfo.c @@ -178,14 +178,21 @@ 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); filename = "(builtin)"; } - if (field != NULL && streq(field, "filename")) { + if (field != NULL && streq(field, "name")) { + printf("%s%c", kmod_module_get_name(mod), + separator); + return 0; + } else if (field != NULL && streq(field, "filename")) { printf("%s%c", filename, separator); return 0; } else if (field == NULL) { + if (is_builtin) { + printf("%-16s%s%c", "name:", + kmod_module_get_name(mod), separator); + } printf("%-16s%s%c", "filename:", filename, separator); } -- 2.28.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] modinfo: dont print module name when --field is given 2020-08-26 9:36 ` Xaver Hörl @ 2020-08-26 10:12 ` Lucas De Marchi 0 siblings, 0 replies; 4+ messages in thread From: Lucas De Marchi @ 2020-08-26 10:12 UTC (permalink / raw) To: Xaver Hörl; +Cc: linux-modules On Wed, Aug 26, 2020 at 2:36 AM Xaver Hörl <hoe.dom@gmx.de> wrote: > > On Tue, Aug 25, 2020 at 10:27:46AM -0700, Lucas De Marchi wrote: > > 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 > > Dropping the "name:" part with --field seems the right thing (updated > patch below implements this). But I don't know what exactly to do about > the error case, not sure if I get your intention right there. > > If --field is not given, the name and filename (i.e. "(builtin)" in > this case) will be there anyway, on the other hand if --field I was thinking about this specific error condition: if (is_builtin && err == -ENOENT) { /* * This is an old kernel that does not have a file * with information about built-in modules. */ return 0; } I.e. if --field is not given and the kernel is old enough not to contain /lib/modules/$(uname -r)/modules.builtin.modinfo, then we print name + filename (builtin). $ modinfo unix name: unix filename: (builtin) alias: net-pf-1 license: GPL file: net/unix/unix $ sudo mv ll $ modinfo unix name: unix filename: (builtin) Lucas De Marchi > _is_ given it seems a bit inconsistent to give additional output. > Unless you consider this case an error condition of sorts, but the > return value being 0 suggests otherwise. > > > Thanks for having a look at this! > Xaver Hörl > > --- > tools/modinfo.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tools/modinfo.c b/tools/modinfo.c > index 0231bb0..b49f622 100644 > --- a/tools/modinfo.c > +++ b/tools/modinfo.c > @@ -178,14 +178,21 @@ 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); > filename = "(builtin)"; > } > > - if (field != NULL && streq(field, "filename")) { > + if (field != NULL && streq(field, "name")) { > + printf("%s%c", kmod_module_get_name(mod), > + separator); > + return 0; > + } else if (field != NULL && streq(field, "filename")) { > printf("%s%c", filename, separator); > return 0; > } else if (field == NULL) { > + if (is_builtin) { > + printf("%-16s%s%c", "name:", > + kmod_module_get_name(mod), separator); > + } > printf("%-16s%s%c", "filename:", > filename, separator); > } > -- > 2.28.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-26 10:12 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-23 21:54 [PATCH] modinfo: dont print module name when --field is given Xaver Hörl 2020-08-25 17:27 ` Lucas De Marchi 2020-08-26 9:36 ` Xaver Hörl 2020-08-26 10:12 ` Lucas De Marchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).