[RFC,v3,08/26] dev: move unrelated macros from header
Commit Message
RTE_FUNC_PTR_OR_* macros have nothing to do with the rte_device object
and associated API.
Move them to rte_common.h and include it where needed.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
drivers/common/qat/qat_device.c | 1 +
drivers/compress/qat/qat_comp_pmd.c | 1 +
drivers/crypto/scheduler/rte_cryptodev_scheduler.c | 1 +
drivers/net/ixgbe/rte_pmd_ixgbe.c | 1 +
drivers/net/liquidio/lio_ethdev.c | 1 +
lib/compressdev/rte_compressdev.c | 1 +
lib/dmadev/rte_dmadev.c | 1 +
lib/eal/include/rte_common.h | 11 +++++++++++
lib/eal/include/rte_dev.h | 11 -----------
lib/ethdev/ethdev_driver.c | 1 +
lib/ethdev/ethdev_pci.h | 1 +
lib/mempool/rte_mempool_ops.c | 1 +
lib/regexdev/rte_regexdev.c | 1 +
lib/security/rte_security.c | 1 +
lib/vhost/vdpa.c | 1 +
15 files changed, 24 insertions(+), 11 deletions(-)
Comments
On Thu, Jul 28, 2022 at 05:26:22PM +0200, David Marchand wrote:
> RTE_FUNC_PTR_OR_* macros have nothing to do with the rte_device object
> and associated API.
> Move them to rte_common.h and include it where needed.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
Personally, I really don't like these macros at all. I think having the
check explicitly in the code would be far more readable, and would only be
one line of code longer than the macro call. Is there some private header
file we could move these to instead of rte_common.h so we can drop their
use in future if we decide to?
On Thu, Jul 28, 2022 at 6:38 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Jul 28, 2022 at 05:26:22PM +0200, David Marchand wrote:
> > RTE_FUNC_PTR_OR_* macros have nothing to do with the rte_device object
> > and associated API.
> > Move them to rte_common.h and include it where needed.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
>
> Personally, I really don't like these macros at all. I think having the
> check explicitly in the code would be far more readable, and would only be
> one line of code longer than the macro call. Is there some private header
> file we could move these to instead of rte_common.h so we can drop their
> use in future if we decide to?
I don't like them either.
Not sure where to put them though...
My "grep-all" shows no user in the projects consuming DPDK I track.
We could mark those macros deprecated, fix our code and drop them later.
On Thu, Jul 28, 2022 at 09:32:38PM +0200, David Marchand wrote:
> On Thu, Jul 28, 2022 at 6:38 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Thu, Jul 28, 2022 at 05:26:22PM +0200, David Marchand wrote:
> > > RTE_FUNC_PTR_OR_* macros have nothing to do with the rte_device object
> > > and associated API.
> > > Move them to rte_common.h and include it where needed.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> >
> > Personally, I really don't like these macros at all. I think having the
> > check explicitly in the code would be far more readable, and would only be
> > one line of code longer than the macro call. Is there some private header
> > file we could move these to instead of rte_common.h so we can drop their
> > use in future if we decide to?
>
> I don't like them either.
> Not sure where to put them though...
>
> My "grep-all" shows no user in the projects consuming DPDK I track.
> We could mark those macros deprecated, fix our code and drop them later.
>
+1 to that.
Can they be marked as deprecated as part of the move perhaps (assuming we
get agreement to kill them?)
On Fri, Jul 29, 2022 at 11:59 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> > > Personally, I really don't like these macros at all. I think having the
> > > check explicitly in the code would be far more readable, and would only be
> > > one line of code longer than the macro call. Is there some private header
> > > file we could move these to instead of rte_common.h so we can drop their
> > > use in future if we decide to?
> >
> > I don't like them either.
> > Not sure where to put them though...
> >
> > My "grep-all" shows no user in the projects consuming DPDK I track.
> > We could mark those macros deprecated, fix our code and drop them later.
> >
> +1 to that.
> Can they be marked as deprecated as part of the move perhaps (assuming we
> get agreement to kill them?)
Yes.
I'll wait for others to chime in, which means this will wait for after
my days off :-).
On Fri, Jul 29, 2022 at 3:22 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Fri, Jul 29, 2022 at 11:59 AM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> > > > Personally, I really don't like these macros at all. I think having the
> > > > check explicitly in the code would be far more readable, and would only be
> > > > one line of code longer than the macro call. Is there some private header
> > > > file we could move these to instead of rte_common.h so we can drop their
> > > > use in future if we decide to?
> > >
> > > I don't like them either.
> > > Not sure where to put them though...
> > >
> > > My "grep-all" shows no user in the projects consuming DPDK I track.
> > > We could mark those macros deprecated, fix our code and drop them later.
> > >
> > +1 to that.
> > Can they be marked as deprecated as part of the move perhaps (assuming we
> > get agreement to kill them?)
Let's see if techboard members have an opinion.
>
> Yes.
> I'll wait for others to chime in, which means this will wait for after
> my days off :-).
24/08/2022 08:50, David Marchand:
> On Fri, Jul 29, 2022 at 3:22 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Fri, Jul 29, 2022 at 11:59 AM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > > > > Personally, I really don't like these macros at all. I think having the
> > > > > check explicitly in the code would be far more readable, and would only be
> > > > > one line of code longer than the macro call. Is there some private header
> > > > > file we could move these to instead of rte_common.h so we can drop their
> > > > > use in future if we decide to?
> > > >
> > > > I don't like them either.
> > > > Not sure where to put them though...
> > > >
> > > > My "grep-all" shows no user in the projects consuming DPDK I track.
> > > > We could mark those macros deprecated, fix our code and drop them later.
> > > >
> > > +1 to that.
> > > Can they be marked as deprecated as part of the move perhaps (assuming we
> > > get agreement to kill them?)
>
> Let's see if techboard members have an opinion.
These macros have no added value for an external user.
I think it is OK to mark them deprecated and plan for a future removal.
Copy of the code for the context:
/** Macros to check for invalid function pointers. */
#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
if ((func) == NULL) \
return retval; \
} while (0)
#define RTE_FUNC_PTR_OR_RET(func) do { \
if ((func) == NULL) \
return; \
} while (0)
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, 24 August 2022 09.39
>
> 24/08/2022 08:50, David Marchand:
> > On Fri, Jul 29, 2022 at 3:22 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > On Fri, Jul 29, 2022 at 11:59 AM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > > > > Personally, I really don't like these macros at all. I think
> having the
> > > > > > check explicitly in the code would be far more readable, and
> would only be
> > > > > > one line of code longer than the macro call. Is there some
> private header
> > > > > > file we could move these to instead of rte_common.h so we can
> drop their
> > > > > > use in future if we decide to?
> > > > >
> > > > > I don't like them either.
> > > > > Not sure where to put them though...
> > > > >
> > > > > My "grep-all" shows no user in the projects consuming DPDK I
> track.
> > > > > We could mark those macros deprecated, fix our code and drop
> them later.
> > > > >
> > > > +1 to that.
> > > > Can they be marked as deprecated as part of the move perhaps
> (assuming we
> > > > get agreement to kill them?)
> >
> > Let's see if techboard members have an opinion.
>
> These macros have no added value for an external user.
> I think it is OK to mark them deprecated and plan for a future removal.
+1 from a community member :-)
>
> Copy of the code for the context:
>
> /** Macros to check for invalid function pointers. */
> #define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
> if ((func) == NULL) \
> return retval; \
> } while (0)
>
> #define RTE_FUNC_PTR_OR_RET(func) do { \
> if ((func) == NULL) \
> return; \
> } while (0)
>
>
>
24/08/2022 13:52, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 24/08/2022 08:50, David Marchand:
> > > On Fri, Jul 29, 2022 at 3:22 PM David Marchand
> > > <david.marchand@redhat.com> wrote:
> > > > > > I don't like them either.
> > > > > > Not sure where to put them though...
> > > > > >
> > > > > > My "grep-all" shows no user in the projects consuming DPDK I
> > track.
> > > > > > We could mark those macros deprecated, fix our code and drop
> > them later.
> > > > > >
> > > > > +1 to that.
> > > > > Can they be marked as deprecated as part of the move perhaps
> > (assuming we
> > > > > get agreement to kill them?)
> > >
> > > Let's see if techboard members have an opinion.
> >
> > These macros have no added value for an external user.
> > I think it is OK to mark them deprecated and plan for a future removal.
>
> +1 from a community member :-)
As usual, your voice is very valuable Morten :)
@@ -2,6 +2,7 @@
* Copyright(c) 2018-2022 Intel Corporation
*/
+#include <rte_common.h>
#include <rte_string_fns.h>
#include <rte_devargs.h>
#include <ctype.h>
@@ -2,6 +2,7 @@
* Copyright(c) 2015-2022 Intel Corporation
*/
+#include <rte_common.h>
#include <rte_malloc.h>
#include "qat_comp.h"
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: BSD-3-Clause
* Copyright(c) 2017 Intel Corporation
*/
+#include <rte_common.h>
#include <rte_string_fns.h>
#include <rte_reorder.h>
#include <rte_cryptodev.h>
@@ -2,6 +2,7 @@
* Copyright(c) 2010-2017 Intel Corporation
*/
+#include <rte_common.h>
#include <ethdev_driver.h>
#include "base/ixgbe_api.h"
@@ -2,6 +2,7 @@
* Copyright(c) 2017 Cavium, Inc
*/
+#include <rte_common.h>
#include <rte_string_fns.h>
#include <ethdev_driver.h>
#include <ethdev_pci.h>
@@ -6,6 +6,7 @@
#include <stdio.h>
#include <inttypes.h>
+#include <rte_common.h>
#include <rte_string_fns.h>
#include <rte_malloc.h>
#include <rte_eal.h>
@@ -5,6 +5,7 @@
#include <inttypes.h>
+#include <rte_common.h>
#include <rte_eal.h>
#include <rte_lcore.h>
#include <rte_log.h>
@@ -861,6 +861,17 @@ rte_log2_u64(uint64_t v)
/** Number of elements in the array. */
#define RTE_DIM(a) (sizeof (a) / sizeof ((a)[0]))
+/** Macros to check for invalid function pointers. */
+#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
+ if ((func) == NULL) \
+ return retval; \
+} while (0)
+
+#define RTE_FUNC_PTR_OR_RET(func) do { \
+ if ((func) == NULL) \
+ return; \
+} while (0)
+
/**
* Converts a numeric string to the equivalent uint64_t value.
* As well as straight number conversion, also recognises the suffixes
@@ -36,17 +36,6 @@ typedef void (*rte_dev_event_cb_fn)(const char *device_name,
enum rte_dev_event_type event,
void *cb_arg);
-/* Macros to check for invalid function pointers */
-#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
- if ((func) == NULL) \
- return retval; \
-} while (0)
-
-#define RTE_FUNC_PTR_OR_RET(func) do { \
- if ((func) == NULL) \
- return; \
-} while (0)
-
/**
* Device policies.
*/
@@ -2,6 +2,7 @@
* Copyright(c) 2022 Intel Corporation
*/
+#include <rte_common.h>
#include <rte_kvargs.h>
#include <rte_malloc.h>
@@ -10,6 +10,7 @@
extern "C" {
#endif
+#include <rte_common.h>
#include <rte_malloc.h>
#include <rte_pci.h>
#include <rte_bus_pci.h>
@@ -6,6 +6,7 @@
#include <stdio.h>
#include <string.h>
+#include <rte_common.h>
#include <rte_string_fns.h>
#include <rte_mempool.h>
#include <rte_errno.h>
@@ -5,6 +5,7 @@
#include <string.h>
+#include <rte_common.h>
#include <rte_memzone.h>
#include <rte_string_fns.h>
@@ -4,6 +4,7 @@
* Copyright (c) 2020 Samsung Electronics Co., Ltd All Rights Reserved
*/
+#include <rte_common.h>
#include <rte_cryptodev.h>
#include <rte_dev.h>
#include <rte_telemetry.h>
@@ -10,6 +10,7 @@
#include <sys/queue.h>
+#include <rte_common.h>
#include <rte_class.h>
#include <rte_malloc.h>
#include <rte_spinlock.h>