On 13/11/2020 19:06, Alexander Duyck wrote:
> On Thu, Nov 12, 2020 at 7:23 AM Edward Cree <ec...@solarflare.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
