[dpdk-dev,v2,02/13] eventdev: fix errors with strict compilation flags

Message ID 743ea1c06b9307c8fc68a399244b8aeab606878e.1493108423.git.adrien.mazarguil@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Adrien Mazarguil April 25, 2017, 8:29 a.m. UTC
  Exported headers must allow compilation with the strictest flags. This
commit addresses the following errors:

 In file included from build/include/rte_eventdev_pmd.h:55:0,
                  from /tmp/check-includes.sh.25816.c:1:
 build/include/rte_eventdev.h:908:8: error: struct has no named members
    [-Werror=pedantic]
 [...]
 In file included from /tmp/check-includes.sh.25816.c:1:0:
 build/include/rte_eventdev_pmd.h:65:35: error: ISO C does not permit named
    variadic macros [-Werror=variadic-macros]
 [...]

Also enabling RTE_LIBRTE_EVENTDEV_DEBUG causes redefinitions:

 In file included from /tmp/check-includes.sh.18921.c:27:0:
    build/include/rte_eventdev_pmd.h:58:0: error: "RTE_PMD_DEBUG_TRACE"
    redefined [-Werror]
 [...]

Rely on the rte_ethdev.h version instead.

Fixes: 71f238432865 ("eventdev: introduce event driven programming model")
Fixes: 4f0804bbdfb9 ("eventdev: implement the northbound APIs")

Cc: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 lib/librte_eventdev/rte_eventdev.h     |  3 +--
 lib/librte_eventdev/rte_eventdev_pmd.h | 24 ++++++++++--------------
 2 files changed, 11 insertions(+), 16 deletions(-)
  

Comments

De Lara Guarch, Pablo April 25, 2017, 3:31 p.m. UTC | #1
Hi Adrien,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Tuesday, April 25, 2017 9:30 AM
> To: dev@dpdk.org
> Cc: Jerin Jacob
> Subject: [dpdk-dev] [PATCH v2 02/13] eventdev: fix errors with strict
> compilation flags
> 
> Exported headers must allow compilation with the strictest flags. This
> commit addresses the following errors:
> 
>  In file included from build/include/rte_eventdev_pmd.h:55:0,
>                   from /tmp/check-includes.sh.25816.c:1:
>  build/include/rte_eventdev.h:908:8: error: struct has no named members
>     [-Werror=pedantic]
>  [...]
>  In file included from /tmp/check-includes.sh.25816.c:1:0:
>  build/include/rte_eventdev_pmd.h:65:35: error: ISO C does not permit
> named
>     variadic macros [-Werror=variadic-macros]
>  [...]
> 
> Also enabling RTE_LIBRTE_EVENTDEV_DEBUG causes redefinitions:
> 
>  In file included from /tmp/check-includes.sh.18921.c:27:0:
>     build/include/rte_eventdev_pmd.h:58:0: error:
> "RTE_PMD_DEBUG_TRACE"
>     redefined [-Werror]
>  [...]
> 
> Rely on the rte_ethdev.h version instead.
> 
> Fixes: 71f238432865 ("eventdev: introduce event driven programming
> model")
> Fixes: 4f0804bbdfb9 ("eventdev: implement the northbound APIs")
> 
> Cc: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

I am seeing the following error due to this patch:
In file included from /root/tmp/dpdk-17.05-rc2/lib/librte_eventdev/rte_eventdev.c:62:0:
/root/tmp/dpdk-17.05-rc2/lib/librte_eventdev/rte_eventdev_pmd.h:54:24: fatal error: rte_ethdev.h: No such file or directory
 #include <rte_ethdev.h>

It only happens when I compile with "-j", so it looks like dependencies have to be fixed?

Thanks,
Pablo
  
Adrien Mazarguil April 26, 2017, 7:06 a.m. UTC | #2
On Tue, Apr 25, 2017 at 03:31:45PM +0000, De Lara Guarch, Pablo wrote:
> Hi Adrien,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Tuesday, April 25, 2017 9:30 AM
> > To: dev@dpdk.org
> > Cc: Jerin Jacob
> > Subject: [dpdk-dev] [PATCH v2 02/13] eventdev: fix errors with strict
> > compilation flags
> > 
> > Exported headers must allow compilation with the strictest flags. This
> > commit addresses the following errors:
> > 
> >  In file included from build/include/rte_eventdev_pmd.h:55:0,
> >                   from /tmp/check-includes.sh.25816.c:1:
> >  build/include/rte_eventdev.h:908:8: error: struct has no named members
> >     [-Werror=pedantic]
> >  [...]
> >  In file included from /tmp/check-includes.sh.25816.c:1:0:
> >  build/include/rte_eventdev_pmd.h:65:35: error: ISO C does not permit
> > named
> >     variadic macros [-Werror=variadic-macros]
> >  [...]
> > 
> > Also enabling RTE_LIBRTE_EVENTDEV_DEBUG causes redefinitions:
> > 
> >  In file included from /tmp/check-includes.sh.18921.c:27:0:
> >     build/include/rte_eventdev_pmd.h:58:0: error:
> > "RTE_PMD_DEBUG_TRACE"
> >     redefined [-Werror]
> >  [...]
> > 
> > Rely on the rte_ethdev.h version instead.
> > 
> > Fixes: 71f238432865 ("eventdev: introduce event driven programming
> > model")
> > Fixes: 4f0804bbdfb9 ("eventdev: implement the northbound APIs")
> > 
> > Cc: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> 
> I am seeing the following error due to this patch:
> In file included from /root/tmp/dpdk-17.05-rc2/lib/librte_eventdev/rte_eventdev.c:62:0:
> /root/tmp/dpdk-17.05-rc2/lib/librte_eventdev/rte_eventdev_pmd.h:54:24: fatal error: rte_ethdev.h: No such file or directory
>  #include <rte_ethdev.h>
> 
> It only happens when I compile with "-j", so it looks like dependencies have to be fixed?

You're right, actually rte_eventdev_pmd.h is not even supposed to have a
dependency on rte_ethdev.h. The problem comes from RTE_FUNC_PTR_OR_ERR_RET()
defined in rte_dev.h, itself relying on the RTE_PMD_DEBUG_TRACE() macro
without any kind of dependency.

This makes no sense, RTE_PMD_DEBUG_TRACE() should be defined along with
rte_pmd_debug_trace() in rte_dev.h. I'll move this fix to a separate commit
to remove all duplicate definitions.
  

Patch

diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index b8ed6ef..20e7293 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -905,9 +905,9 @@  rte_event_dev_close(uint8_t dev_id);
  * The generic *rte_event* structure to hold the event attributes
  * for dequeue and enqueue operation
  */
+RTE_STD_C11
 struct rte_event {
 	/** WORD0 */
-	RTE_STD_C11
 	union {
 		uint64_t event;
 		/** Event attributes for dequeue or enqueue operation */
@@ -967,7 +967,6 @@  struct rte_event {
 		};
 	};
 	/** WORD1 */
-	RTE_STD_C11
 	union {
 		uint64_t u64;
 		/**< Opaque 64-bit value */
diff --git a/lib/librte_eventdev/rte_eventdev_pmd.h b/lib/librte_eventdev/rte_eventdev_pmd.h
index a73dc91..a090fa4 100644
--- a/lib/librte_eventdev/rte_eventdev_pmd.h
+++ b/lib/librte_eventdev/rte_eventdev_pmd.h
@@ -51,27 +51,23 @@  extern "C" {
 #include <rte_malloc.h>
 #include <rte_log.h>
 #include <rte_common.h>
+#include <rte_ethdev.h>
 
 #include "rte_eventdev.h"
 
-#ifdef RTE_LIBRTE_EVENTDEV_DEBUG
-#define RTE_PMD_DEBUG_TRACE(...) \
-	rte_pmd_debug_trace(__func__, __VA_ARGS__)
-#else
-#define RTE_PMD_DEBUG_TRACE(...)
-#endif
-
 /* Logging Macros */
-#define RTE_EDEV_LOG_ERR(fmt, args...) \
-	RTE_LOG(ERR, EVENTDEV, "%s() line %u: " fmt "\n",  \
-			__func__, __LINE__, ## args)
+#define RTE_EDEV_LOG_ERR(...) \
+	RTE_LOG(ERR, EVENTDEV, \
+		RTE_FMT("%s() line %u: " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \
+			__func__, __LINE__, RTE_FMT_TAIL(__VA_ARGS__,)))
 
 #ifdef RTE_LIBRTE_EVENTDEV_DEBUG
-#define RTE_EDEV_LOG_DEBUG(fmt, args...) \
-	RTE_LOG(DEBUG, EVENTDEV, "%s() line %u: " fmt "\n",  \
-			__func__, __LINE__, ## args)
+#define RTE_EDEV_LOG_DEBUG(...) \
+	RTE_LOG(DEBUG, EVENTDEV, \
+		RTE_FMT("%s() line %u: " RTE_FMT_HEAD(__VA_ARGS__,) "\n", \
+			__func__, __LINE__, RTE_FMT_TAIL(__VA_ARGS__,)))
 #else
-#define RTE_EDEV_LOG_DEBUG(fmt, args...) (void)0
+#define RTE_EDEV_LOG_DEBUG(...) (void)0
 #endif
 
 /* Macros to check for valid device */