On 25 April 2017 at 09:59, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolai...@nokia-bell-labs.com> wrote:
> > > > -----Original Message----- > > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of > Maxim > > Uvarov > > Sent: Monday, April 24, 2017 10:36 PM > > To: lng-odp@lists.linaro.org > > Subject: Re: [lng-odp] [API-NEXT PATCH v2 2/8] linux-gen: cpu_flags: > added > > x86 cpu flag read functions > > > > ERROR: Macros with complex values should be enclosed in parentheses > > #239: FILE: platform/linux-generic/arch/x86/cpu_flags.c:172: > > +#define FEAT_DEF(name, leaf, subleaf, reg, bit) \ > > + [RTE_CPUFLAG_##name] = {leaf, subleaf, reg, bit, #name }, > > CHECK: architecture specific defines should be avoided > > #346: FILE: platform/linux-generic/arch/x86/cpu_flags.c:279: > > +#if defined(__i386__) && defined(__PIC__) > > total: 1 errors, 0 warnings, 1 checks, 403 lines checked > > > > First one looks like possible to correct. But it's copy pasted code. > > Maybe better to have original code to more easy apply updates. > > > > Maxim. > > > > > Yes, it a complex macro but it's used to initialize array elements: > > static const struct feature_entry cpu_feature_table[] = { > FEAT_DEF(SSE3, 0x00000001, 0, RTE_REG_ECX, 0) > FEAT_DEF(PCLMULQDQ, 0x00000001, 0, RTE_REG_ECX, 1) > FEAT_DEF(DTES64, 0x00000001, 0, RTE_REG_ECX, 2) > > For example, adding extra () around ... > > [RTE_CPUFLAG_SSE3] = {0x00000001, 0, RTE_REG_ECX, 0, SSE3} > > ... would not make it more robust or readable. In fact, it would not even > compile: > > error: expected expression before '[' token > int foo[] = {([0] = 100), ([1] = 200)}; > > > -Petri > > > > Yes, that is used to declare array of structs. Parentheses can not be added there. A little bit not clear why dpdk folks put comma to macro. Without comma code looks better for my taste: #define FEAT_DEF(name, leaf, subleaf, reg, bit) \ - [RTE_CPUFLAG_##name] = {leaf, subleaf, reg, bit, #name }, + [RTE_CPUFLAG_##name] = {leaf, subleaf, reg, bit, #name } static const struct feature_entry cpu_feature_table[] = { + FEAT_DEF(SSE3, 0x00000001, 0, RTE_REG_ECX, 0), + FEAT_DEF(PCLMULQDQ, 0x00000001, 0, RTE_REG_ECX, 1), + FEAT_DEF(DTES64, 0x00000001, 0, RTE_REG_ECX, 2), But I think it's reasonable to keep original code to more easy patch this file with upstream fixes. Maxim.
