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

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----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