netdev - Re: [PATCH net-next 1/3] sfc: extend bitfield macros to 19 fields

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <72c7dac3-9744-0006-b859-50b4e3ccf5bf@solarflare.com>
Date:   Mon, 16 Nov 2020 12:26:54 +0000
From:   Edward Cree <ecree@...arflare.com>
To:     Alexander Duyck <alexander.duyck@...il.com>
CC:     Solarflare linux maintainers <linux-net-drivers@...arflare.com>,
        "Jakub Kicinski" <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/3] sfc: extend bitfield macros to 19 fields

On 13/11/2020 19:06, Alexander Duyck wrote:
> On Thu, Nov 12, 2020 at 7:23 AM Edward Cree <ecree@...arflare.com> wrote:
>> @@ -348,7 +352,11 @@ typedef union efx_oword {
>>  #endif
>>
>>  /* Populate an octword field with various numbers of arguments */
>> -#define EFX_POPULATE_OWORD_17 EFX_POPULATE_OWORD
>> +#define EFX_POPULATE_OWORD_19 EFX_POPULATE_OWORD
>> +#define EFX_POPULATE_OWORD_18(oword, ...) \
>> +       EFX_POPULATE_OWORD_19(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
>> +#define EFX_POPULATE_OWORD_17(oword, ...) \
>> +       EFX_POPULATE_OWORD_18(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
>>  #define EFX_POPULATE_OWORD_16(oword, ...) \
>>         EFX_POPULATE_OWORD_17(oword, EFX_DUMMY_FIELD, 0, __VA_ARGS__)
>>  #define EFX_POPULATE_OWORD_15(oword, ...) \
> Are all these macros really needed? It seems like this is adding a
> bunch of noise in order to add support for a few additional fields.
> Wouldn't it be possible to just define the ones that are actually
> needed and add multiple dummy values to fill in the gaps instead of
> defining every macro between zero and 19? For example this patch set
> adds an option for setting 18 fields, but from what I can tell it is
> never used.
I guess the reasoningoriginally was that it's easier to read and
 v-lint if it's just n repetitions of the same pattern.  Whereas if
 there were jumps, it'd be more likely for a typo to slip through
 unnoticed and subtly corrupt all the values.
But tbh I don't know, it's been like that since the driver was added
 twelve years ago (8ceee660aacb) when it had all from 0 to 10.  All
 we've done since then is extend that pattern.

-ed