net/mlx5: fix compilation issue with gcc pragma

Message ID 1569928223-6600-1-git-send-email-viacheslavo@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: fix compilation issue with gcc pragma |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-compilation success Compile Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Slava Ovsiienko Oct. 1, 2019, 11:10 a.m. UTC
  Some compilers (i.e Intel icc) do not recognize GCC diagnostic
pragma, the compiler check is added.

Fixes: a46a42b5cd03 ("net/mlx5: add VF LAG mode bonding device recognition")

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
  

Comments

Stephen Hemminger Oct. 1, 2019, 2:54 p.m. UTC | #1
On Tue,  1 Oct 2019 11:10:23 +0000
Viacheslav Ovsiienko <viacheslavo@mellanox.com> wrote:

> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> +#pragma GCC diagnostic push
>  #pragma GCC diagnostic ignored "-Wformat-nonliteral"
> +#endif
> +	/* Use safe format to check maximal buffer length. */
>  	while (fscanf(file, format, ifname) == 1) {
> -#pragma GCC diagnostic error "-Wformat-nonliteral"
> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> +#pragma GCC diagnostic pop
> +#endif

This is messy, is there not a better way to do this?
  
Slava Ovsiienko Oct. 1, 2019, 5:15 p.m. UTC | #2
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, October 1, 2019 17:54
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh <rasland@mellanox.com>; ferruh.yigit@intel.com
> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with gcc
> pragma
> 
> On Tue,  1 Oct 2019 11:10:23 +0000
> Viacheslav Ovsiienko <viacheslavo@mellanox.com> wrote:
> 
> > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> #pragma GCC
> > +diagnostic push
> >  #pragma GCC diagnostic ignored "-Wformat-nonliteral"
> > +#endif
> > +	/* Use safe format to check maximal buffer length. */
> >  	while (fscanf(file, format, ifname) == 1) { -#pragma GCC diagnostic
> > error "-Wformat-nonliteral"
> > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> #pragma GCC
> > +diagnostic pop #endif
> 
> This is messy, is there not a better way to do this?

At least I did not find one.

The GCC compile-time format checking feature is nice in general and it worth 
to be engaged. The legitimate fscanf() usage with variable format parameter
causes GCC to emit error/warning, so we should suppress these ones for this
single line. ICC does not emit warning and does not recognize GCC pragmas.
Clang just does not recognize fscanf().

Should we use "#ifndef __INTEL_COMPILER" (typical workaround for
GCC diagnostic pragma in DPDK)? I'm not sure, It is not completely correct.

The alternative I see is to implement dedicated routine to read words from the file,
but it means more code and more run-time resources. It seems not to be 
the right way to push compile-time issues resolving to the run-time.

Defining the macro is not relevant here because this is a single case.

WBR, Slava
  
Stephen Hemminger Oct. 1, 2019, 11:41 p.m. UTC | #3
On Tue, 1 Oct 2019 17:15:46 +0000
Slava Ovsiienko <viacheslavo@mellanox.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Tuesday, October 1, 2019 17:54
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> > Darawsheh <rasland@mellanox.com>; ferruh.yigit@intel.com
> > Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with gcc
> > pragma
> > 
> > On Tue,  1 Oct 2019 11:10:23 +0000
> > Viacheslav Ovsiienko <viacheslavo@mellanox.com> wrote:
> >   
> > > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)  
> > #pragma GCC  
> > > +diagnostic push
> > >  #pragma GCC diagnostic ignored "-Wformat-nonliteral"
> > > +#endif
> > > +	/* Use safe format to check maximal buffer length. */
> > >  	while (fscanf(file, format, ifname) == 1) { -#pragma GCC diagnostic
> > > error "-Wformat-nonliteral"
> > > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)  
> > #pragma GCC  
> > > +diagnostic pop #endif  
> > 
> > This is messy, is there not a better way to do this?  
> 
> At least I did not find one.
> 
> The GCC compile-time format checking feature is nice in general and it worth 
> to be engaged. The legitimate fscanf() usage with variable format parameter
> causes GCC to emit error/warning, so we should suppress these ones for this
> single line. ICC does not emit warning and does not recognize GCC pragmas.
> Clang just does not recognize fscanf().
> 
> Should we use "#ifndef __INTEL_COMPILER" (typical workaround for
> GCC diagnostic pragma in DPDK)? I'm not sure, It is not completely correct.
> 
> The alternative I see is to implement dedicated routine to read words from the file,
> but it means more code and more run-time resources. It seems not to be 
> the right way to push compile-time issues resolving to the run-time.
> 
> Defining the macro is not relevant here because this is a single case.
> 
> WBR, Slava
> 
> 

You are going to a lot of effort to solve a problem of interface name length
which can not happen.  The maximum interface name in linux and bsd is always
15 characters plus null. Therefore there is no need to build a dynamic
format string at all here. Or you could use the assignment allocation modifier
so that the resulting string from fscanf was allocated.

Could you try one of these instead.
  
Slava Ovsiienko Oct. 2, 2019, 6:15 a.m. UTC | #4
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, October 2, 2019 2:41
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh <rasland@mellanox.com>; ferruh.yigit@intel.com
> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with gcc
> pragma
> 
> On Tue, 1 Oct 2019 17:15:46 +0000
> Slava Ovsiienko <viacheslavo@mellanox.com> wrote:
> 
> > > -----Original Message-----
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > Sent: Tuesday, October 1, 2019 17:54
> > > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > > Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh
> > > <rasland@mellanox.com>; ferruh.yigit@intel.com
> > > Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with
> > > gcc pragma
> > >
> > > On Tue,  1 Oct 2019 11:10:23 +0000
> > > Viacheslav Ovsiienko <viacheslavo@mellanox.com> wrote:
> > >
> > > > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> > > #pragma GCC
> > > > +diagnostic push
> > > >  #pragma GCC diagnostic ignored "-Wformat-nonliteral"
> > > > +#endif
> > > > +	/* Use safe format to check maximal buffer length. */
> > > >  	while (fscanf(file, format, ifname) == 1) { -#pragma GCC
> > > > diagnostic error "-Wformat-nonliteral"
> > > > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> > > #pragma GCC
> > > > +diagnostic pop #endif
> > >
> > > This is messy, is there not a better way to do this?
> >
> > At least I did not find one.
> >
> > The GCC compile-time format checking feature is nice in general and it
> > worth to be engaged. The legitimate fscanf() usage with variable
> > format parameter causes GCC to emit error/warning, so we should
> > suppress these ones for this single line. ICC does not emit warning and does
> not recognize GCC pragmas.
> > Clang just does not recognize fscanf().
> >
> > Should we use "#ifndef __INTEL_COMPILER" (typical workaround for GCC
> > diagnostic pragma in DPDK)? I'm not sure, It is not completely correct.
> >
> > The alternative I see is to implement dedicated routine to read words
> > from the file, but it means more code and more run-time resources. It
> > seems not to be the right way to push compile-time issues resolving to the
> run-time.
> >
> > Defining the macro is not relevant here because this is a single case.
> >
> > WBR, Slava
> >
> >
> 
> You are going to a lot of effort to solve a problem of interface name length
> which can not happen.  The maximum interface name in linux and bsd is
> always 15 characters plus null.

We just have a definition IF_NAMESIZE. If we have the definition - we should follow, right?

> Therefore there is no need to build a dynamic format
> string at all here. Or you could use the assignment allocation modifier so that
> the resulting string from fscanf was allocated.

The allocation modifier has questionable compatibility either,
does involve implicit memory allocations and requires explicit free call.
It seems to be less robust than using a standard length modifier.

> 
> Could you try one of these instead.
It seems there is better solution - stringification,
please see:  http://patches.dpdk.org/patch/60415/
I like stringification not too much, but it seems there is the right place to use one.

WBR, Slava
  
Slava Ovsiienko Oct. 2, 2019, 6:55 a.m. UTC | #5
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Slava Ovsiienko
> Sent: Wednesday, October 2, 2019 9:15
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh <rasland@mellanox.com>; ferruh.yigit@intel.com
> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with gcc
> pragma
> 
> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Wednesday, October 2, 2019 2:41
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh
> > <rasland@mellanox.com>; ferruh.yigit@intel.com
> > Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with
> > gcc pragma
> >
> > On Tue, 1 Oct 2019 17:15:46 +0000
> > Slava Ovsiienko <viacheslavo@mellanox.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > Sent: Tuesday, October 1, 2019 17:54
> > > > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > > > Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> > Darawsheh
> > > > <rasland@mellanox.com>; ferruh.yigit@intel.com
> > > > Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue
> > > > with gcc pragma
> > > >
> > > > On Tue,  1 Oct 2019 11:10:23 +0000 Viacheslav Ovsiienko
> > > > <viacheslavo@mellanox.com> wrote:
> > > >
> > > > > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> > > > #pragma GCC
> > > > > +diagnostic push
> > > > >  #pragma GCC diagnostic ignored "-Wformat-nonliteral"
> > > > > +#endif
> > > > > +	/* Use safe format to check maximal buffer length. */
> > > > >  	while (fscanf(file, format, ifname) == 1) { -#pragma GCC
> > > > > diagnostic error "-Wformat-nonliteral"
> > > > > +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> > > > #pragma GCC
> > > > > +diagnostic pop #endif
> > > >
> > > > This is messy, is there not a better way to do this?
> > >
> > > At least I did not find one.
> > >
> > > The GCC compile-time format checking feature is nice in general and
> > > it worth to be engaged. The legitimate fscanf() usage with variable
> > > format parameter causes GCC to emit error/warning, so we should
> > > suppress these ones for this single line. ICC does not emit warning
> > > and does
> > not recognize GCC pragmas.
> > > Clang just does not recognize fscanf().
> > >
> > > Should we use "#ifndef __INTEL_COMPILER" (typical workaround for GCC
> > > diagnostic pragma in DPDK)? I'm not sure, It is not completely correct.
> > >
> > > The alternative I see is to implement dedicated routine to read
> > > words from the file, but it means more code and more run-time
> > > resources. It seems not to be the right way to push compile-time
> > > issues resolving to the
> > run-time.
> > >
> > > Defining the macro is not relevant here because this is a single case.
> > >
> > > WBR, Slava
> > >
> > >
> >
> > You are going to a lot of effort to solve a problem of interface name
> > length which can not happen.  The maximum interface name in linux and
> > bsd is always 15 characters plus null.
> 
> We just have a definition IF_NAMESIZE. If we have the definition - we should
> follow, right?
> 
> > Therefore there is no need to build a dynamic format string at all
> > here. Or you could use the assignment allocation modifier so that the
> > resulting string from fscanf was allocated.
> 
> The allocation modifier has questionable compatibility either, does involve
> implicit memory allocations and requires explicit free call.
> It seems to be less robust than using a standard length modifier.
> 
> >
> > Could you try one of these instead.
> It seems there is better solution - stringification, please see:
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatche
> s.dpdk.org%2Fpatch%2F60415%2F&amp;data=02%7C01%7Cviacheslavo%40
> mellanox.com%7Cfad3629b2e694dde62f908d746ffe45a%7Ca652971c7d2e4
> d9ba6a4d149256f461b%7C0%7C0%7C637055937198169033&amp;sdata=vx
> EXTvYh12PJdU9ZmlqAzIThILKmAG23r4OyKE0DBvU%3D&amp;reserved=0
> I like stringification not too much, but it seems there is the right place to use
> one.

Also, I would add something like this:

assert(atol(STRINGIFY(IF_NAMESIZE)) == IF_NAMESIZE);

to make sure IF_NAMESIZE is not defined like as "BASESIZE+1".
What do you think ?

WBR, Slava
  
Ferruh Yigit Oct. 2, 2019, 11:54 a.m. UTC | #6
On 10/2/2019 7:55 AM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Slava Ovsiienko
>> Sent: Wednesday, October 2, 2019 9:15
>> To: Stephen Hemminger <stephen@networkplumber.org>
>> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
>> Darawsheh <rasland@mellanox.com>; ferruh.yigit@intel.com
>> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with gcc
>> pragma
>>
>>> -----Original Message-----
>>> From: Stephen Hemminger <stephen@networkplumber.org>
>>> Sent: Wednesday, October 2, 2019 2:41
>>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
>>> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
>> Darawsheh
>>> <rasland@mellanox.com>; ferruh.yigit@intel.com
>>> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with
>>> gcc pragma
>>>
>>> On Tue, 1 Oct 2019 17:15:46 +0000
>>> Slava Ovsiienko <viacheslavo@mellanox.com> wrote:
>>>
>>>>> -----Original Message-----
>>>>> From: Stephen Hemminger <stephen@networkplumber.org>
>>>>> Sent: Tuesday, October 1, 2019 17:54
>>>>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
>>>>> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
>>> Darawsheh
>>>>> <rasland@mellanox.com>; ferruh.yigit@intel.com
>>>>> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue
>>>>> with gcc pragma
>>>>>
>>>>> On Tue,  1 Oct 2019 11:10:23 +0000 Viacheslav Ovsiienko
>>>>> <viacheslavo@mellanox.com> wrote:
>>>>>
>>>>>> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
>>>>> #pragma GCC
>>>>>> +diagnostic push
>>>>>>  #pragma GCC diagnostic ignored "-Wformat-nonliteral"
>>>>>> +#endif
>>>>>> +	/* Use safe format to check maximal buffer length. */
>>>>>>  	while (fscanf(file, format, ifname) == 1) { -#pragma GCC
>>>>>> diagnostic error "-Wformat-nonliteral"
>>>>>> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
>>>>> #pragma GCC
>>>>>> +diagnostic pop #endif
>>>>>
>>>>> This is messy, is there not a better way to do this?
>>>>
>>>> At least I did not find one.
>>>>
>>>> The GCC compile-time format checking feature is nice in general and
>>>> it worth to be engaged. The legitimate fscanf() usage with variable
>>>> format parameter causes GCC to emit error/warning, so we should
>>>> suppress these ones for this single line. ICC does not emit warning
>>>> and does
>>> not recognize GCC pragmas.
>>>> Clang just does not recognize fscanf().
>>>>
>>>> Should we use "#ifndef __INTEL_COMPILER" (typical workaround for GCC
>>>> diagnostic pragma in DPDK)? I'm not sure, It is not completely correct.
>>>>
>>>> The alternative I see is to implement dedicated routine to read
>>>> words from the file, but it means more code and more run-time
>>>> resources. It seems not to be the right way to push compile-time
>>>> issues resolving to the
>>> run-time.
>>>>
>>>> Defining the macro is not relevant here because this is a single case.
>>>>
>>>> WBR, Slava
>>>>
>>>>
>>>
>>> You are going to a lot of effort to solve a problem of interface name
>>> length which can not happen.  The maximum interface name in linux and
>>> bsd is always 15 characters plus null.
>>
>> We just have a definition IF_NAMESIZE. If we have the definition - we should
>> follow, right?
>>
>>> Therefore there is no need to build a dynamic format string at all
>>> here. Or you could use the assignment allocation modifier so that the
>>> resulting string from fscanf was allocated.
>>
>> The allocation modifier has questionable compatibility either, does involve
>> implicit memory allocations and requires explicit free call.
>> It seems to be less robust than using a standard length modifier.
>>
>>>
>>> Could you try one of these instead.
>> It seems there is better solution - stringification, please see:
>> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatche
>> s.dpdk.org%2Fpatch%2F60415%2F&amp;data=02%7C01%7Cviacheslavo%40
>> mellanox.com%7Cfad3629b2e694dde62f908d746ffe45a%7Ca652971c7d2e4
>> d9ba6a4d149256f461b%7C0%7C0%7C637055937198169033&amp;sdata=vx
>> EXTvYh12PJdU9ZmlqAzIThILKmAG23r4OyKE0DBvU%3D&amp;reserved=0
>> I like stringification not too much, but it seems there is the right place to use
>> one.

+1, this is better than the pragma

But there is already 'RTE_STR' for stringify, can you please use it?


> 
> Also, I would add something like this:
> 
> assert(atol(STRINGIFY(IF_NAMESIZE)) == IF_NAMESIZE);
> 
> to make sure IF_NAMESIZE is not defined like as "BASESIZE+1".
> What do you think ?

I think fscanf() will give a build error in that case, so may not need assertion.

> 
> WBR, Slava
>
  
Slava Ovsiienko Oct. 2, 2019, 12:40 p.m. UTC | #7
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, October 2, 2019 14:54
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh <rasland@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with gcc
> pragma
> 
> On 10/2/2019 7:55 AM, Slava Ovsiienko wrote:
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Slava Ovsiienko
> >> Sent: Wednesday, October 2, 2019 9:15
> >> To: Stephen Hemminger <stephen@networkplumber.org>
> >> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> Darawsheh
> >> <rasland@mellanox.com>; ferruh.yigit@intel.com
> >> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with
> >> gcc pragma
> >>
> >>> -----Original Message-----
> >>> From: Stephen Hemminger <stephen@networkplumber.org>
> >>> Sent: Wednesday, October 2, 2019 2:41
> >>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> >>> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> >> Darawsheh
> >>> <rasland@mellanox.com>; ferruh.yigit@intel.com
> >>> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue with
> >>> gcc pragma
> >>>
> >>> On Tue, 1 Oct 2019 17:15:46 +0000
> >>> Slava Ovsiienko <viacheslavo@mellanox.com> wrote:
> >>>
> >>>>> -----Original Message-----
> >>>>> From: Stephen Hemminger <stephen@networkplumber.org>
> >>>>> Sent: Tuesday, October 1, 2019 17:54
> >>>>> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> >>>>> Cc: dev@dpdk.org; Matan Azrad <matan@mellanox.com>; Raslan
> >>> Darawsheh
> >>>>> <rasland@mellanox.com>; ferruh.yigit@intel.com
> >>>>> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix compilation issue
> >>>>> with gcc pragma
> >>>>>
> >>>>> On Tue,  1 Oct 2019 11:10:23 +0000 Viacheslav Ovsiienko
> >>>>> <viacheslavo@mellanox.com> wrote:
> >>>>>
> >>>>>> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> >>>>> #pragma GCC
> >>>>>> +diagnostic push
> >>>>>>  #pragma GCC diagnostic ignored "-Wformat-nonliteral"
> >>>>>> +#endif
> >>>>>> +	/* Use safe format to check maximal buffer length. */
> >>>>>>  	while (fscanf(file, format, ifname) == 1) { -#pragma GCC
> >>>>>> diagnostic error "-Wformat-nonliteral"
> >>>>>> +#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
> >>>>> #pragma GCC
> >>>>>> +diagnostic pop #endif
> >>>>>
> >>>>> This is messy, is there not a better way to do this?
> >>>>
> >>>> At least I did not find one.
> >>>>
> >>>> The GCC compile-time format checking feature is nice in general and
> >>>> it worth to be engaged. The legitimate fscanf() usage with variable
> >>>> format parameter causes GCC to emit error/warning, so we should
> >>>> suppress these ones for this single line. ICC does not emit warning
> >>>> and does
> >>> not recognize GCC pragmas.
> >>>> Clang just does not recognize fscanf().
> >>>>
> >>>> Should we use "#ifndef __INTEL_COMPILER" (typical workaround for
> >>>> GCC diagnostic pragma in DPDK)? I'm not sure, It is not completely
> correct.
> >>>>
> >>>> The alternative I see is to implement dedicated routine to read
> >>>> words from the file, but it means more code and more run-time
> >>>> resources. It seems not to be the right way to push compile-time
> >>>> issues resolving to the
> >>> run-time.
> >>>>
> >>>> Defining the macro is not relevant here because this is a single case.
> >>>>
> >>>> WBR, Slava
> >>>>
> >>>>
> >>>
> >>> You are going to a lot of effort to solve a problem of interface
> >>> name length which can not happen.  The maximum interface name in
> >>> linux and bsd is always 15 characters plus null.
> >>
> >> We just have a definition IF_NAMESIZE. If we have the definition - we
> >> should follow, right?
> >>
> >>> Therefore there is no need to build a dynamic format string at all
> >>> here. Or you could use the assignment allocation modifier so that
> >>> the resulting string from fscanf was allocated.
> >>
> >> The allocation modifier has questionable compatibility either, does
> >> involve implicit memory allocations and requires explicit free call.
> >> It seems to be less robust than using a standard length modifier.
> >>
> >>>
> >>> Could you try one of these instead.
> >> It seems there is better solution - stringification, please see:
> >>
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatc
> >> he
> >>
> s.dpdk.org%2Fpatch%2F60415%2F&amp;data=02%7C01%7Cviacheslavo%40
> >>
> mellanox.com%7Cfad3629b2e694dde62f908d746ffe45a%7Ca652971c7d2e4
> >>
> d9ba6a4d149256f461b%7C0%7C0%7C637055937198169033&amp;sdata=vx
> >> EXTvYh12PJdU9ZmlqAzIThILKmAG23r4OyKE0DBvU%3D&amp;reserved=0
> >> I like stringification not too much, but it seems there is the right
> >> place to use one.
> 
> +1, this is better than the pragma
> 
> But there is already 'RTE_STR' for stringify, can you please use it?
Thanks for the clue, I did not find it with "grep stringi".

> 
> 
 >
> > Also, I would add something like this:
> >
> > assert(atol(STRINGIFY(IF_NAMESIZE)) == IF_NAMESIZE);
> >
> > to make sure IF_NAMESIZE is not defined like as "BASESIZE+1".
> > What do you think ?
> 
> I think fscanf() will give a build error in that case, so may not need assertion.
Not always, something like "#define IF_NAMESIZE data_len" passes
the compiler check, so I've added assert.

With best regards, Slava
  

Patch

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 951b9f5..7a3f654 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2296,11 +2296,15 @@  struct mlx5_dev_spawn_data {
 	if (!file)
 		return -1;
 	MKSTR(format, "%c%us", '%', (unsigned int)(sizeof(ifname) - 1));
-
-	/* Use safe format to check maximal buffer length. */
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
+#pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wformat-nonliteral"
+#endif
+	/* Use safe format to check maximal buffer length. */
 	while (fscanf(file, format, ifname) == 1) {
-#pragma GCC diagnostic error "-Wformat-nonliteral"
+#if defined(RTE_TOOLCHAIN_GCC) && (GCC_VERSION >= 40600)
+#pragma GCC diagnostic pop
+#endif
 		char tmp_str[IF_NAMESIZE + 32];
 		struct rte_pci_addr pci_addr;
 		struct mlx5_switch_info	info;