[dpdk-dev] [PATCH] net/mlx5: fix compilation issue with gcc pragma

[Slava Ovsiienko]

> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: Wednesday, October 2, 2019 2:41
> To: Slava Ovsiienko <viacheslavo at mellanox.com>
> Cc: dev at dpdk.org; Matan Azrad <matan at mellanox.com>; Raslan
> Darawsheh <rasland at mellanox.com>; ferruh.yigit at 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 at mellanox.com> wrote:
> 
> > > -----Original Message-----
> > > From: Stephen Hemminger <stephen at networkplumber.org>
> > > Sent: Tuesday, October 1, 2019 17:54
> > > To: Slava Ovsiienko <viacheslavo at mellanox.com>
> > > Cc: dev at dpdk.org; Matan Azrad <matan at mellanox.com>; Raslan
> Darawsheh
> > > <rasland at mellanox.com>; ferruh.yigit at 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 at 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