Alan Jenkins napsal(a):
> On 9/10/09, Michal Marek <[email protected]> wrote:
>> +    while ((line = getline_wrapped(f, &linenum)) != NULL) {
>> +            char *module = strrchr(line, '/');
>> +
>> +            if (!*line || *line == '#') {
>> +                    free(line);
>> +                    continue;
>> +            }
> 
>> +            module = strrchr(line, '/');
> 
> this duplicates the assignment in the declaration of module.

Oops.


>  But I
> wonder why this whole chunk of code is needed...
> 
>> +            if (module)
>> +                    module++;
>> +            else
>> +                    module = line;
>> +            if (ends_in(module, ".ko"))
>> +                    module[strlen(module) - 3] = '\0';
>> +            underscores(module);
> 
> ...because we already have filename2modname().  I think you can just
> do filename2modname(module, module).

Cool. Thanks for reviewing this.



> Let's see, if mit_remove is set, we treat it as a normal module, find
> that it is not present, and return success.  "modprobe -r --first-time
> $builtin-module" will fail as expected... but the error message will
> be wrong
> 
> "FATAL: Module $builtin-module is not in kernel."
> 
> How about this (not tested, may exceed 80 cols):

Maybe it's good time to move it to a function :).

> 
>                       if (!aliases &&
>                           module_builtin(dirname, modname) == 1) {
>                               if (flags & mit_remove) {
>                                       if (flags & mit_first_time)
>                                               error("Module %s is builtin\n", 
> modname);
>                                       return 1;

I think --first-time shouldn't make a difference when removing a builtin
module, I would consider modprobe -r <builtin-mod> an error.


>                               } else if (flags & mit_first_time) {
>                                       error("Module %s already in kernel 
> (builtin).\n",
>                                             modname);
>                                       return 1;
>                               } else if (flags & mit_ignore_loaded) {
>                                       /* --show-depends given */
>                                       info("builtin %s\n", modname);
>                               }
>                               return 0;
>                       }
> 
> Regards
> Alan

--
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