On Sat, Feb 4, 2012 at 11:36 AM, Lucas De Marchi
<lucas.demarchi@xxxxxxxxxxxxxx> wrote:
> 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.
Since I was in a hurry for next release, I amended the patch myself.
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
