modinfo: various small fixes.

Improve Android support (see code comments).

Remove unnecessary fixed-length limits.

Show error if module not found (plus test).

Expand help text.

Switch to FLAG macro.

Stop hard-coding module assumptions in the tests.

2 files changed, 58 insertions, 25 deletions

diff --git a/tests/modinfo.test b/tests/modinfo.test
index 0a8c2bec..f39dc6ff 100644
--- a/tests/modinfo.test
+++ b/tests/modinfo.test
@@ -4,7 +4,23 @@
 
 #testing "name" "command" "result" "infile" "stdin"
 
-[ -e /proc/modules ] || { echo "Skipping test because modules are not supported"; exit 1; }
+testcmd "missing" "missing 2>&1" "modinfo: missing: not found\n" "" ""
+
+[ -e /proc/modules ] || { echo "Skipping: no /proc/modules"; exit 1; }
+
+# Android keeps its modules on the vendor partition.
+MODULE_ROOT=""
+[ -d /vendor/lib/modules ] && MODULE_ROOT="/vendor"
+
+# Find some modules to work with.
+MODULE_PATH1=$(find $MODULE_ROOT/lib/modules -name *.ko | head -1 2>/dev/null)
+MODULE1=$(basename -s .ko $MODULE_PATH1)
+MODULE_PATH2=$(find $MODULE_ROOT/lib/modules -name *.ko | tail -1 2>/dev/null)
+MODULE2=$(basename -s .ko $MODULE_PATH2)
+DASH_MODULE=$(basename -s .ko \
+  $(find $MODULE_ROOT/lib/modules -name *-*.ko | tail -1 2>/dev/null))
+BAR_MODULE=$(basename -s .ko \
+  $(find $MODULE_ROOT/lib/modules -name *_*.ko | tail -1 2>/dev/null))
 
 # modinfo does not need to output fields in a specified order.
 # Instead, there are labelled fields.  We can use sort to make up for this.
@@ -12,19 +28,19 @@
 # which change from kernel to kernel and can be disabled. 
 # We grep to remove these.
 
-#We expect they have ne2k-pci as a module.
-
-testing "gets right number of fields" "modinfo ne2k-pci |cut -d: -f1 |grep -v ver|sort" "alias\nalias\nalias\nalias\nalias\nalias\nalias\nalias\nalias\nalias\nalias\nauthor\ndepends\ndescription\nfilename\nlicense\nparm\nparm\nparm\n" "" ""
-testing "treats - and _ as equivalent" "modinfo ne2k_pci |cut -d: -f1 |grep -v ver|sort" "alias\nalias\nalias\nalias\nalias\nalias\nalias\nalias\nalias\nalias\nalias\nauthor\ndepends\ndescription\nfilename\nlicense\nparm\nparm\nparm\n" "" ""
+skipnot [ -n "$DASH_MODULE" ]
+testing "treats - and _ as equivalent" "modinfo $DASH_MODULE > dash-dash &&
+  modinfo ${DASH_MODULE/-/_} > dash-bar && diff -u dash-dash dash-bar" "" "" ""
+skipnot [ -n "$BAR_MODULE" ]
+testing "treats _ and - as equivalent" "modinfo $BAR_MODULE > bar-bar &&
+  modinfo ${BAR_MODULE/_/-} > bar-dash && diff -u bar-bar bar-dash" "" "" ""
 
 # Output of -F filename should be an absolute path to the module.
 # Otherwise, initrd generating scripts will break.
 
-testing "-F filename gets absolute path" "[ -e `modinfo -F filename ne2k-pci` ] && echo ne2k-pci " "ne2k-pci\n" "" ""
-
-testing "supports multiple modules" "modinfo -F filename ne2k-pci 8390 | wc -l" "2\n" "" ""
-
-testing "does not output filename for bad module" "modinfo -F filename zxcvbnm__9753" "" "" ""
-
-
+testing "-F filename gets absolute path" "modinfo -F filename $MODULE1" \
+  "$MODULE_PATH1\n" "" ""
 
+skipnot [ "$MODULE1" != "$MODULE2" ]
+testing "supports multiple modules" "modinfo -F filename $MODULE1 $MODULE2" \
+  "$MODULE_PATH1\n$MODULE_PATH2\n" "" ""
diff --git a/toys/other/modinfo.c b/toys/other/modinfo.c
index eaf6cb98..286570f1 100644
--- a/toys/other/modinfo.c
+++ b/toys/other/modinfo.c
@@ -10,10 +10,14 @@ config MODINFO
   bool "modinfo"
   default y
   help
-    usage: modinfo [-0] [-b basedir] [-k kernrelease] [-F field] [modulename...]
+    usage: modinfo [-0] [-b basedir] [-k kernel] [-F field] [module|file...]
 
-    Display module fields for all specified modules, looking in
-    <basedir>/lib/modules/<kernrelease>/ (kernrelease defaults to uname -r).
+    Display module fields for modules specified by name or .ko path.
+
+    -F  Only show the given field
+    -0  Separate fields with NUL rather than newline
+    -b  Use <basedir> as root for /lib/modules/
+    -k  Look in given directory under /lib/modules/
 */
 
 #define FOR_modinfo
@@ -23,6 +27,7 @@ GLOBALS(
   char *F, *k, *b;
 
   long mod;
+  int count;
 )
 
 static void output_field(char *field, char *value)
@@ -30,7 +35,7 @@ static void output_field(char *field, char *value)
   if (!TT.F) xprintf("%s:%*c", field, 15-(int)strlen(field), ' ');
   else if (strcmp(TT.F, field)) return;
   xprintf("%s", value);
-  xputc((toys.optflags & FLAG_0) ? 0 : '\n');
+  xputc(FLAG(0) ? 0 : '\n');
 }
 
 static void modinfo_file(char *full_name)
@@ -53,6 +58,7 @@ static void modinfo_file(char *full_name)
     return;
   }
 
+  TT.count++;
   output_field("filename", full_name);
 
   for (pos = buf; pos < buf+len; pos++) {
@@ -100,19 +106,30 @@ static int check_module(struct dirtree *new)
 
 void modinfo_main(void)
 {
-  for(TT.mod = 0; TT.mod<toys.optc; TT.mod++) {
+  struct utsname uts;
+
+  // Android (as shipped by Google) currently only has modules on /vendor.
+  // Android does not support multiple sets of modules for different kernels.
+  if (CFG_TOYBOX_ON_ANDROID) {
+   if (!TT.b) TT.b = "/vendor";
+   if (!TT.k) TT.k = "";
+  } else {
+   uname(&uts);
+   if (!TT.b) TT.b = "";
+   if (!TT.k) TT.k = uts.release;
+  }
+
+  for (TT.mod = 0; TT.mod<toys.optc; TT.mod++) {
     char *s = strstr(toys.optargs[TT.mod], ".ko");
 
     if (s && !s[3]) modinfo_file(toys.optargs[TT.mod]);
     else {
-      struct utsname uts;
-
-      if (uname(&uts) < 0) perror_exit("bad uname");
-      if (snprintf(toybuf, sizeof(toybuf), "%s/lib/modules/%s",
-          (toys.optflags & FLAG_b) ? TT.b : "",
-          (toys.optflags & FLAG_k) ? TT.k : uts.release) >= sizeof(toybuf))
-            perror_exit("basedir/kernrelease too long");
-      dirtree_read(toybuf, check_module);
+      char *path = xmprintf("%s/lib/modules/%s", TT.b, TT.k);
+
+      TT.count = 0;
+      dirtree_read(path, check_module);
+      if (!TT.count) error_msg("%s: not found", toys.optargs[TT.mod]);
+      free(path);
     }
   }
 }