Hi Lucas,

Thank you for review.

24.11.2012 01:00, Lucas De Marchi пишет:

I don't like the strstr loop above. We could either

1) check the extensions similarly to what  depmod does
2) rely on user passing "--" for module names. This was my preferred
from the beginning but this could break current users.

I think (1) could be very simple.


From 94a0a34073899244fc3f666ef0ff5d22ad39f0ec Mon Sep 17 00:00:00 2001
From: Aleksey Makarov <[email protected]>
Date: Sat, 24 Nov 2012 18:55:13 +0700
Subject: [PATCH] fix is_module_filename()

modinfo fails if there is a ".ko" substring in the path to the module
---
 libkmod/libkmod-util.c | 25 +++++++++++++++++++++++++
 libkmod/libkmod-util.h |  8 ++++++++
 tools/depmod.c         | 40 ++--------------------------------------
 tools/modinfo.c        | 10 ++--------
 4 files changed, 37 insertions(+), 46 deletions(-)

diff --git a/libkmod/libkmod-util.c b/libkmod/libkmod-util.c
index b5a2588..05a26cc 100644
--- a/libkmod/libkmod-util.c
+++ b/libkmod/libkmod-util.c
@@ -308,6 +308,31 @@ char *path_make_absolute_cwd(const char *p)
        return r;
 }
+const struct kmod_ext kmod_exts[] = {
+       {".ko", sizeof(".ko") - 1},
+#ifdef ENABLE_ZLIB
+       {".ko.gz", sizeof(".ko.gz") - 1},
+#endif
+#ifdef ENABLE_XZ
+       {".ko.xz", sizeof(".ko.xz") - 1},
+#endif
+       {NULL, 0},
+};
+
+int path_ends_with_kmod_ext(const char *path, size_t len)
+{
+       const struct kmod_ext *eitr;
+
+       for (eitr = kmod_exts; eitr->ext != NULL; eitr++) {
+               if (len <= eitr->len)
+                       continue;
+               if (streq(path + len - eitr->len, eitr->ext))
+                       return true;
+       }
+
+       return false;
+}
+
 #define USEC_PER_SEC  1000000ULL
 #define NSEC_PER_USEC 1000ULL
 unsigned long long ts_usec(const struct timespec *ts)
diff --git a/libkmod/libkmod-util.h b/libkmod/libkmod-util.h
index 5fe3e02..36192d9 100644
--- a/libkmod/libkmod-util.h
+++ b/libkmod/libkmod-util.h
@@ -22,6 +22,14 @@ char *path_make_absolute_cwd(const char *p) _must_check_ 
__attribute__((nonnull(
 int alias_normalize(const char *alias, char buf[PATH_MAX], size_t *len) 
_must_check_ __attribute__((nonnull(1,2)));
 char *modname_normalize(const char *modname, char buf[PATH_MAX], size_t *len) 
__attribute__((nonnull(1, 2)));
 char *path_to_modname(const char *path, char buf[PATH_MAX], size_t *len) 
__attribute__((nonnull(2)));
+
+extern const struct kmod_ext {
+       const char *ext;
+       size_t len;
+} kmod_exts[];
+#define KMOD_EXT_UNC 0
+int path_ends_with_kmod_ext(const char *path, size_t len) 
__attribute__((nonnull(1)));
+
 unsigned long long stat_mstamp(const struct stat *st);
 unsigned long long ts_usec(const struct timespec *ts);
diff --git a/tools/depmod.c b/tools/depmod.c
index 7bbdcd3..de77391 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -40,22 +40,6 @@
 #define DEFAULT_VERBOSE LOG_WARNING
 static int verbose = DEFAULT_VERBOSE;
-#define KMOD_EXT_UNC 0
-
-static const struct kmod_ext {
-       const char *ext;
-       size_t len;
-} kmod_exts[] = {
-       {".ko", sizeof(".ko") - 1},
-#ifdef ENABLE_ZLIB
-       {".ko.gz", sizeof(".ko.gz") - 1},
-#endif
-#ifdef ENABLE_XZ
-       {".ko.xz", sizeof(".ko.xz") - 1},
-#endif
-       {NULL, 0},
-};
-
 static const char CFG_BUILTIN_KEY[] = "built-in";
 static const char *default_cfg_paths[] = {
        "/run/depmod.d",
@@ -1204,20 +1188,10 @@ static int depmod_modules_search_file(struct depmod 
*depmod, size_t baselen, siz
        struct mod *mod;
        const char *relpath;
        char modname[PATH_MAX];
-       const struct kmod_ext *eitr;
        size_t modnamelen;
-       uint8_t matches = 0;
        int err;
- for (eitr = kmod_exts; eitr->ext != NULL; eitr++) {
-               if (namelen <= eitr->len)
-                       continue;
-               if (streq(path + baselen + namelen - eitr->len, eitr->ext)) {
-                       matches = 1;
-                       break;
-               }
-       }
-       if (!matches)
+       if (!path_ends_with_kmod_ext(path, baselen + namelen))
                return 0;
if (path_to_modname(path, modname, &modnamelen) == NULL) {
@@ -2413,17 +2387,7 @@ static int depfile_up_to_date_dir(DIR *d, time_t mtime, 
size_t baselen, char *pa
                                                     path);
                        closedir(subdir);
                } else if (S_ISREG(st.st_mode)) {
-                       const struct kmod_ext *eitr;
-                       uint8_t matches = 0;
-                       for (eitr = kmod_exts; eitr->ext != NULL; eitr++) {
-                               if (namelen <= eitr->len)
-                                       continue;
-                               if (streq(name + namelen - eitr->len, 
eitr->ext)) {
-                                       matches = 1;
-                                       break;
-                               }
-                       }
-                       if (!matches)
+                       if (!path_ends_with_kmod_ext(path, namelen))
                                continue;
                        memcpy(path + baselen, name, namelen + 1);
                        err = st.st_mtime <= mtime;
diff --git a/tools/modinfo.c b/tools/modinfo.c
index aec2608..17ed50d 100644
--- a/tools/modinfo.c
+++ b/tools/modinfo.c
@@ -27,6 +27,7 @@
 #include <sys/utsname.h>
 #include <sys/stat.h>
 #include "libkmod.h"
+#include "libkmod-util.h"
#include "kmod.h" @@ -347,17 +348,10 @@ static void help(void)
 static bool is_module_filename(const char *name)
 {
        struct stat st;
-       const char *ptr;
if (stat(name, &st) == 0 && S_ISREG(st.st_mode) &&
-                                       (ptr = strstr(name, ".ko")) != NULL) {
-               /*
-                * We screened for .ko; make sure this is either at the end of
-                * the name or followed by another '.' (e.g. gz or xz modules)
-                */
-               if(ptr[3] == '\0' || ptr[3] == '.')
+               path_ends_with_kmod_ext(name, strlen(name)))
                        return true;
-       }
return false;
 }
--
1.8.0

--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to