On 9/10/09, Michal Marek <mmarek@xxxxxxx> wrote:
> Alan Jenkins napsal(a):
>> On 9/10/09, Michal Marek <mmarek@xxxxxxx> 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.
Agreed
--
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
