Hi Ferruh,
On Tue, Oct 16, 2018 at 04:55:43PM +0100, Ferruh Yigit wrote:
> On 10/12/2018 3:54 PM, Stephen Hemminger wrote:
> > On Fri, 12 Oct 2018 14:43:57 +0200
> > Adrien Mazarguil <adrien.mazarg...@6wind.com> wrote:
> >
> >> On Fri, Oct 12, 2018 at 11:45:01AM +0100, Ferruh Yigit wrote:
> >>> On 10/12/2018 11:42 AM, Ferruh Yigit wrote:
> >>>> On 10/11/2018 6:59 PM, Stephen Hemminger wrote:
> >>>>> @@ -161,8 +161,9 @@ extern "C" {
> >>>>>
> >>>>> extern int rte_eth_dev_logtype;
> >>>>>
> >>>>> -#define RTE_ETHDEV_LOG(level, ...) \
> >>>>> - rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, "" __VA_ARGS__)
> >>>>> +#define RTE_ETHDEV_LOG(level, fmt, ...) \
> >>>>> + rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, \
> >>>>> + "%s():" fmt, __func__, ## __VA_ARGS__)
> >>>>
> >>>> +1 to adding function name, but
> >>>>
> >>>> failsafe is giving build error [1] with clang because of ## usage [2],
> >>>> that is
> >>>> why I add this as ` "" __VA_ARGS__` at first place but you can't do this
> >>>> trick
> >>>> if __VA_ARGS__ used after fmt.
> >>>>
> >>>> I am not aware of a solution for this, __VA_OPT__(,) also didn't worked
> >>>> with clang.
> >>>
> >>> +cc Adrien & Gaetan,
> >>>
> >>> I saw Adrien put some "workaround" to this for mlx5
> >>
> >> Yes, through RTE_FMT() (rte_common.h). Something like this:
> >>
> >> #define RTE_ETHDEV_LOG(level, fmt, ...) \
> >> rte_log(RTE_LOG_ ## level, \
> >> rte_eth_dev_logtype, \
> >> "%s():" fmt, \
> >> __func__, \
> >> ## __VA_ARGS__)
> >>
> >> Can be rewritten like that:
> >>
> >> #define RTE_ETHDEV_LOG(level, ...) \
> >> rte_log(RTE_LOG_ ## level, \
> >> rte_eth_dev_logtype, \
> >> RTE_FMT("%s():" RTE_FMT_HEAD(__VA_ARGS__,), \
> >> __func__, \
> >> RTE_FMT_TAIL(__VA_ARGS__,)))
> >>
> >> Although not too pretty and convenient, it does the job. In short:
> >>
> >> - Remove "fmt" argument from prototype.
> >> - Enclose format string and its arguments in RTE_FMT().
> >> - Replace "fmt" with RTE_FMT_HEAD(__VA_ARGS__,).
> >> - Replace "## __VA_ARGS__" with RTE_FMT_TAIL(__VA_ARGS__,).
> >> - Yes, trailing commas are mandatory in RTE_FMT_(HEAD|TAIL)().
> >> - Note it quietly appends a dummy "%.0s" argument to the format string.
> >>
> >>>> [1]
> >>>> .../build/include/rte_ethdev.h:166:26: error: token pasting of ',' and
> >>>> __VA_ARGS__ is a GNU extension
> >>>> [-Werror,-Wgnu-zero-variadic-macro-arguments]
> >>>> "%s():" fmt, __func__, ## __VA_ARGS__)
> >>>> ^
> >>>>
> >>>> [2]
> >>>> This seems because of "-pedantic" argument driver uses, and other PMDs
> >>>> using
> >>>> "-pedantic", like mlx, will have same error although they are disable by
> >>>> default and error not observed in default build.
> >>>>
> >>>
> >>
> >
> > Since zero varadic macros is a GNU extension, maybe just adding GNU source?
>
> I think `-pedantic` is preventing using extension whether we have
> `_GNU_SOURCE`
> or not.
>
> Gaetan,
>
> Why we have `-pedantic` option enabled for failsafe?
Because I wanted to enforce it in failsafe, and core DPDK headers were
compatible so it wasn't too much of a hassle.
If rte_ethdev.h is meant to lose this compatibility, then pedantic will
be disabled upon including it. But I guess if it was kept compatible
until now, then it was a deliberate effort?
--
Gaëtan Rivet
6WIND
