[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH 1/2] modprobe: allow blacklist for compatibility
From:       Felipe Contreras <felipe.contreras () gmail ! com>
Date:       2012-01-31 15:40:40
Message-ID: CAMP44s3w15PCi2EZHeXDoxTmzzX5fYeE9imn0xH7TO=rb40MuQ () mail ! gmail ! com
[Download RAW message or body]

On Tue, Jan 31, 2012 at 5:14 PM, Denys Vlasenko
<vda.linux@googlemail.com> wrote:
> On Tue, Jan 31, 2012 at 3:46 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>>> This change in effect removed FEATURE_MODPROBE_BLACKLIST.
>>> It still exists, but nothing depends on it.
>>
>> Huh? It's still there:
>>
>>                } else if (ENABLE_FEATURE_MODPROBE_BLACKLIST
>>                 && strcmp(tokens[0], "blacklist") == 0
>>                ) {
>>                        /* blacklist <modulename> */
>>                        get_or_add_modentry(tokens[1])->flags |= MODULE_FLAG_BLACKLISTED;
>>                }
>>
>> So -b would do nothing if FEATURE_MODPROBE_BLACKLIST is not enabled.
>
> I see. You're right.
>
>> But it would show on the options and everything, and the help is clear:
>>
>>  Apply blacklist to module names too (if supported)"
>
> Usually I don't include no-op options into help text.
> I only include those which actually work.

Up to you. I don't see any problem either way.

>>> You should not change OPT_BLACKLIST.
>>
>> Why not? It would not have any effect anyway.
>
>
>                        if (!(opt & OPT_BLACKLIST)
>                         || !(me->flags & MODULE_FLAG_BLACKLISTED)
>                        ) {
>                                rc |= do_modprobe(me);
>                        }
>
> To not do a useless check here ^^^^^^^^^^^

Hardly an issue worth considering.

But up to you.

-- 
Felipe Contreras
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]