Re: [dpdk-dev] [PATCH v7] eal: remove sys/queue.h from public headers.

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

2021-08-18 23:26 (UTC+0000), William Tu:
[...]
> diff --git a/lib/eal/linux/include/rte_os.h b/lib/eal/linux/include/rte_os.h
> index 1618b4df22..ce5b0aed52 100644
> --- a/lib/eal/linux/include/rte_os.h
> +++ b/lib/eal/linux/include/rte_os.h
> @@ -11,6 +11,21 @@
>   */
>  
>  #include <sched.h>
> +#include <sys/queue.h>
> +
> +/* These macros are compatible with system's sys/queue.h. */
> +#define RTE_TAILQ_HEAD(name, type) TAILQ_HEAD(name, type)
> +#define RTE_TAILQ_ENTRY(type) TAILQ_ENTRY(type)
> +#define RTE_TAILQ_FOREACH(var, head, field) TAILQ_FOREACH(var, head, field)
> +#define RTE_TAILQ_FOREACH_SAFE(var, head, field, tvar) \
> +	for ((var) = TAILQ_FIRST((head)); \
> +	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
> +	    (var) = (tvar))

I made a comment to v5 and you were going to fix it, maybe it got lost?

	Why duplicate this in rte_os.h (documentation lost, BTW) and add #ifdef?
	RTE_TAILQ_FOREACH_SAFE is not needed in headers, it can be left
	[in rte_tailq.h].

The important part is duplication in rte_os.h for each platform
and the loss of documentation. I see you already removed #ifdef.

> +#define RTE_TAILQ_FIRST(head) TAILQ_FIRST(head)
> +#define RTE_TAILQ_NEXT(elem, field) TAILQ_NEXT(elem, field)
> +#define RTE_STAILQ_HEAD(name, type) STAILQ_HEAD(name, type)
> +#define RTE_STAILQ_ENTRY(type) STAILQ_ENTRY(type)
> +
>  
>  #ifdef CPU_SETSIZE /* may require _GNU_SOURCE */
>  typedef cpu_set_t rte_cpuset_t;
> diff --git a/lib/eal/windows/eal_alarm.c b/lib/eal/windows/eal_alarm.c
> index e5dc54efb8..103c1f909d 100644
> --- a/lib/eal/windows/eal_alarm.c
> +++ b/lib/eal/windows/eal_alarm.c
> @@ -4,6 +4,7 @@
>  
>  #include <stdatomic.h>
>  #include <stdbool.h>
> +#include <sys/queue.h>
>  
>  #include <rte_alarm.h>
>  #include <rte_spinlock.h>
> diff --git a/lib/eal/windows/include/rte_os.h b/lib/eal/windows/include/rte_os.h
> index 66c711d458..0cbe1dbc1e 100644
> --- a/lib/eal/windows/include/rte_os.h
> +++ b/lib/eal/windows/include/rte_os.h
> @@ -18,6 +18,37 @@
>  extern "C" {
>  #endif
>  
> +/* These macros are compatible with bundled sys/queue.h. */
> +#define RTE_TAILQ_HEAD(name, type) \
> +struct name { \
> +	struct type *tqh_first; /* first element */ \
> +	struct type **tqh_last; /* addr of last next element */ \
> +}
> +#define RTE_TAILQ_ENTRY(type) \
> +struct { \
> +	struct type *tqe_next; /* next element */ \
> +	struct type **tqe_prev; /* address of previous next element */ \
> +}
> +#define RTE_TAILQ_FOREACH(var, head, field) \
> +	for ((var) = RTE_TAILQ_FIRST((head)); \
> +	    (var); \
> +	    (var) = RTE_TAILQ_NEXT((var), field))
> +#define RTE_TAILQ_FOREACH_SAFE(var, head, field, tvar) \
> +	for ((var) = TAILQ_FIRST((head)); \
> +	    (var) && ((tvar) = TAILQ_NEXT((var), field), 1); \
> +	    (var) = (tvar))
> +#define RTE_TAILQ_FIRST(head) ((head)->tqh_first)
> +#define RTE_TAILQ_NEXT(elm, field) ((elm)->field.tqe_next)
> +#define RTE_STAILQ_HEAD(name, type) \
> +struct name { \
> +	struct type *stqh_first;/* first element */ \
> +	struct type **stqh_last;/* addr of last next element */ \
> +}
> +#define RTE_STAILQ_ENTRY(type) \
> +struct { \
> +	struct type *stqe_next; /* next element */ \
> +}
> +

Please drop the inline comments.
They duplicate what's already in sys/queue.h
and we're not going to maintain them.