[PATCH] modutils: fix config options dependency (2)

Denys Vlasenko vda.linux at googlemail.com
Tue Feb 7 15:40:52 UTC 2017


On Tue, Feb 7, 2017 at 4:38 PM, Kang-Che Sung <explorer09 at gmail.com> wrote:
> On Tue, Feb 7, 2017 at 11:21 PM, Denys Vlasenko
> <vda.linux at googlemail.com> wrote:
>> On Tue, Feb 7, 2017 at 4:38 AM, Kang-Che Sung <explorer09 at gmail.com> wrote:
>>> On Tue, Feb 7, 2017 at 4:56 AM, Denys Vlasenko <vda.linux at googlemail.com> wrote:
>>>>
>>>> Not really. If you would explain it, it might increase chances of it
>>>> being accepted. Since I had to guess, I guessed "it probably saves a few bytes
>>>> of code at the cost of many more #ifdefs.
>>>
>>> Yes, it's to save a few bytes in the generated machine code.
>>> (Although I think of this later I might be putting to many #ifdefs
>>> than necessary.)
>>>
>>> Here this is sufficient:
>>>
>>>     #define is_depmod_or_modprobe \
>>>     ((ENABLE_MODPROBE || ENABLE_DEPMOD) \
>>>     && ((!ENABLE_INSMOD && !ENABLE_RMMOD) \
>>>     || (!ENABLE_MODPROBE && is_depmod) \
>>>     || ((applet_name[0] & '\x04') != 0)))
>>
>> How about this?
>>
>> -       if (is_depmod || is_modprobe) {
>> +       if ((MOD_APPLET_CNT == 2 && ENABLE_MODPROBE && ENABLE_DEPMOD)
>> +        || is_depmod || is_modprobe
>> +       ) {
>
> You can imagine how it will evaluate for this configuration:
>
> #define ENABLE_DEPMOD 1
> #define ENABLE_MODPROBE 1
> #define ENABLE_INSMOD 1
> /* ENABLE_RMMOD is not set */
>
> MOD_APPLET_CNT==3
>
> (MOD_APPLET_CNT == 2) evaluates to false (0)
> is_depmod expands to (applet_name[0]=='d')
> is_modprobe expands to (applet_name[0]=='m')
>
> So the whole expression becomes
> (applet_name[0]=='d' || applet_name[0]=='m')
>
> I would prefer the smaller comparison: (applet_name[0] != 'i')
> Or the equal sized one: (applet_name[0] & '\4')
>
> This is the MOD_APPLET_CNT==3 case that I've thought of.

The difference would be +1 2-byte asm instruction on x86.


More information about the busybox mailing list