diff mbox

[dpdk-dev] cryptodev: fix RTE_PMD_DEBUG_TRACE redefinition

Message ID 1456961677-1116-1-git-send-email-marcdevel@gmail.com (mailing list archive)
State Rejected, archived
Headers

Commit Message

Marc Sune March 2, 2016, 11:34 p.m. UTC 
  RTE_PMD_DEBUG_TRACE used RTE_FUNC_PTR_OR_ERR_RET was redefined
in rte_cryptodev_pmd.h which produced MACRO redefinition warnings
when including both rte_cryptodev_pmd.h and rte_ethdev.h.

This commit moves MACRO definition to rte_cryptodev.c to prevent
this warning.
---
 lib/librte_cryptodev/rte_cryptodev.c     | 7 +++++++
 lib/librte_cryptodev/rte_cryptodev_pmd.h | 7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

Comments

Thomas Monjalon March 10, 2016, 6:23 p.m. UTC | #1 
2016-03-03 00:34, Marc Sune:
> RTE_PMD_DEBUG_TRACE used RTE_FUNC_PTR_OR_ERR_RET was redefined
> in rte_cryptodev_pmd.h which produced MACRO redefinition warnings
> when including both rte_cryptodev_pmd.h and rte_ethdev.h.
> 
> This commit moves MACRO definition to rte_cryptodev.c to prevent
> this warning.

It is not the right fix.

This macro should probably be renamed with a crypto prefix or defined
only once (same thing for ethdev).
The function rte_pmd_debug_trace() and the macros
RTE_PROC_PRIMARY_OR_ERR_RET,
RTE_PROC_PRIMARY_OR_RET,
RTE_FUNC_PTR_OR_ERR_RET,
RTE_FUNC_PTR_OR_RET
should not be in lib/librte_eal/common/include/rte_dev.h.
The macros call RTE_PMD_DEBUG_TRACE which is defined elsewhere.
The rte_log.h is probably a better place.
But why these macros have no PMD prefix?
Marc Sune March 14, 2016, 10:16 p.m. UTC | #2 
On 10 March 2016 at 19:23, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> 2016-03-03 00:34, Marc Sune:
> > RTE_PMD_DEBUG_TRACE used RTE_FUNC_PTR_OR_ERR_RET was redefined
> > in rte_cryptodev_pmd.h which produced MACRO redefinition warnings
> > when including both rte_cryptodev_pmd.h and rte_ethdev.h.
> >
> > This commit moves MACRO definition to rte_cryptodev.c to prevent
> > this warning.
>
> It is not the right fix.
>

This MACRO is only used in that .c file, so it actually makes sense not to
be defined in the header file.


>
> This macro should probably be renamed with a crypto prefix or defined
> only once (same thing for ethdev).
> The function rte_pmd_debug_trace() and the macros
> RTE_PROC_PRIMARY_OR_ERR_RET,
> RTE_PROC_PRIMARY_OR_RET,
> RTE_FUNC_PTR_OR_ERR_RET,
> RTE_FUNC_PTR_OR_RET
> should not be in lib/librte_eal/common/include/rte_dev.h.
> The macros call RTE_PMD_DEBUG_TRACE which is defined elsewhere.
> The rte_log.h is probably a better place.
> But why these macros have no PMD prefix?
>

While I agree, this patch is only trying to solve the compilation error,
which I think it is a step forward from what we have now. I am working on
C++ linking automated test and I could not include both files in the test.

Marc
diff mbox

Patch

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 2838852..90d2c30 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -71,6 +71,13 @@ 
 #include "rte_cryptodev.h"
 #include "rte_cryptodev_pmd.h"
 
+#ifdef RTE_LIBRTE_CRYPTODEV_DEBUG
+#define RTE_PMD_DEBUG_TRACE(...) \
+	rte_pmd_debug_trace(__func__, __VA_ARGS__)
+#else
+#define RTE_PMD_DEBUG_TRACE(fmt, args...)
+#endif
+
 struct rte_cryptodev rte_crypto_devices[RTE_CRYPTO_MAX_DEVS];
 
 struct rte_cryptodev *rte_cryptodevs = &rte_crypto_devices[0];
diff --git a/lib/librte_cryptodev/rte_cryptodev_pmd.h b/lib/librte_cryptodev/rte_cryptodev_pmd.h
index 8270afa..c43680f 100644
--- a/lib/librte_cryptodev/rte_cryptodev_pmd.h
+++ b/lib/librte_cryptodev/rte_cryptodev_pmd.h
@@ -62,13 +62,6 @@  struct rte_cryptodev_qp_conf;
 
 enum rte_cryptodev_event_type;
 
-#ifdef RTE_LIBRTE_CRYPTODEV_DEBUG
-#define RTE_PMD_DEBUG_TRACE(...) \
-	rte_pmd_debug_trace(__func__, __VA_ARGS__)
-#else
-#define RTE_PMD_DEBUG_TRACE(fmt, args...)
-#endif
-
 struct rte_cryptodev_session {
 	struct {
 		uint8_t dev_id;