Hi Dan
* Dan McGee <dan@xxxxxxxxxxxxx> [2012-02-03 20:25:00 -0600]:
> A simple case of breakage before this commit:
>
> $ touch aes
> $ modinfo aes
> filename: /tmp/aes
> ERROR: could not get modinfo from 'aes': Invalid argument
>
We had a similar problem with modprobe and decided removing the path
option. However for modinfo I think what you did is a good idea.
> Add a new is_module_filename() function that attempts to do more than
> just check if the passed argument is a regular file. We look at the name
> for a '.ko' string, and if that is found, ensure it is either at the end
> of the string or followed by another '.' (for .gz and .xz modules, for
> instance). We don't make this second option conditional on the way the
> tools are built with compression support; the file is a module file
> regardless and should always be treated that way.
>
Yes... I just hope there will not be bug reports regardin this. Bare in
mind module aliases may contain dots.
> When doing this, and noticed in the test suite output, we open the
> system modules index unconditionally, even if it is never going to be
> used during the modinfo call, which is the case when passing module
> filenames directly. Delay the opening of the index file until we get an
> argument that is not a module filename.
Go one step further and remove the call to kmod_load_resources().
There's no point in calling it.
>
> With-help-from: Dave Reisner <dreisner@xxxxxxxxxxxxx>
> Signed-off-by: Dan McGee <dan@xxxxxxxxxxxxx>
> ---
> tools/kmod-modinfo.c | 28 ++++++++++++++++++++++++----
> 1 files changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/tools/kmod-modinfo.c b/tools/kmod-modinfo.c
> index 87483a5..bad344c 100644
> --- a/tools/kmod-modinfo.c
> +++ b/tools/kmod-modinfo.c
> @@ -19,6 +19,7 @@
>
> #include <stdio.h>
> #include <stdlib.h>
> +#include <stdbool.h>
> #include <getopt.h>
> #include <errno.h>
> #include <string.h>
> @@ -332,6 +333,21 @@ static void help(const char *progname)
> progname);
> }
>
> +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) */
80 chars?
> + if(ptr[3] != '\0' && ptr[3] != '.')
> + return false;
> + return true;
> + }
> + return false;
> +}
> +
> static int do_modinfo(int argc, char *argv[])
> {
> struct kmod_ctx *ctx;
> @@ -341,6 +357,7 @@ static int do_modinfo(int argc, char *argv[])
> const char *root = NULL;
> const char *null_config = NULL;
> int i, err;
> + bool resources_loaded = false;
>
> for (;;) {
> int c, idx = 0;
> @@ -418,18 +435,21 @@ static int do_modinfo(int argc, char *argv[])
> fputs("Error: kmod_new() failed!\n", stderr);
> return EXIT_FAILURE;
> }
> - kmod_load_resources(ctx);
>
> err = 0;
> for (i = optind; i < argc; i++) {
> const char *name = argv[i];
> - struct stat st;
> int r;
>
> - if (stat(name, &st) == 0 && S_ISREG(st.st_mode))
> + if (is_module_filename(name)) {
> r = modinfo_path_do(ctx, name);
> - else
> + } else {
> + if (!resources_loaded) {
> + kmod_load_resources(ctx);
> + resources_loaded = true;
> + }
As said, no need to kmod_load_resources. The reason is that it's only 1
module that might be using the index, so just let the fallback
implementation setup the necessary things later.
Regards,
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
