| Message ID | 20200826093627.ukw4hwff2wxb6ykh@blumerang (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | modinfo: dont print module name when --field is given | expand |
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 >
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); }