From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id BF3ACA0C58;
	Mon, 23 Aug 2021 14:35:22 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 450094014D;
	Mon, 23 Aug 2021 14:35:22 +0200 (CEST)
Received: from mail-lj1-f176.google.com (mail-lj1-f176.google.com
 [209.85.208.176])
 by mails.dpdk.org (Postfix) with ESMTP id 16B7140042
 for <dev@dpdk.org>; Mon, 23 Aug 2021 14:35:21 +0200 (CEST)
Received: by mail-lj1-f176.google.com with SMTP id s3so31190200ljp.11
 for <dev@dpdk.org>; Mon, 23 Aug 2021 05:35:21 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=mime-version:references:in-reply-to:from:date:message-id:subject:to
 :cc; bh=R3C7XhpFLopmoSEwanbIB3atYDlLkPHIxrcdtlYY/4c=;
 b=hca4ctFBKyntGKLdTJ7LmDCQQ/nd4tZuKDWl2+ZWZD7dc99SSulrL9fvQ113wtuLV9
 j/Md2/WKiRleV3t45RNZayVW4zpqdXvGraxhsNv54gp9NAe2JudjSf60AWuX3CB99+dm
 ion71TQlALifHRg1sgk8XXnTNlraaYkp13QMqO5le/+Q+ID1tSU0agy8wuXzlyek6oaB
 2rdbxmqm6SFSqwx3JXep1Fdwae22xFVJLjd5NxVuo1XIEAvaCzqO3xw9zkNoYIfQyKjI
 oCr/F0pIXtqwLDGJNgrsLriS6hroI3iKZwlW99cq3VsdQxJNtlbawIC70jPhGyqdAg2m
 jOzQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=R3C7XhpFLopmoSEwanbIB3atYDlLkPHIxrcdtlYY/4c=;
 b=OfOm0khmMtgMCCJm+l1EB5K6J1y3gxjfSUU+1KCmEyZRAChmT8IUuCsO5aX6BhT7tK
 A3QLiwri8JIlesXqnOcINuaFidfiLkXagv4GH0e+hfilRiDrMq7t0KkaimLNhPfyVMsS
 3PP17WGYOd+zZOuAxZxr7A02XaXJUB2c6CnO0XAtfVc+KrWaFEzdf4pIP9e2Z1uKYKOw
 3AnXfQtXG3QOcB3O1ZhZx4FzuEJjBn4vx9Al9pd6s8mFp8CbZQLayzb3TliXE00WnbVq
 dNopmoHcSlC7XLbxCfdmEomEIw0aNNcrhOv3lRgy/H3cnUoXNW0W4kNr3FHVVcovFaC2
 rH+w==
X-Gm-Message-State: AOAM533QRkwf3BvRAlpR5L+ziw5tZ0Li+niUNIslmkeiqvZ+XprHtWid
 4GIOwp0VyRzCYF3+FoMYrv0UnrzW4ok6MzeoOQY=
X-Google-Smtp-Source: ABdhPJxEEtf8TpErf7ZNz4knvZMpFCyssMIw5WgTJujTl983Tnfilue8+lTuJjN4QFsmwZec/yEPwHCW+A+iDQVT4kk=
X-Received: by 2002:a2e:b042:: with SMTP id d2mr22027444ljl.279.1629722120522; 
 Mon, 23 Aug 2021 05:35:20 -0700 (PDT)
MIME-Version: 1.0
References: <20210814025154.13430-1-u9012063@gmail.com>
 <20210818232632.7068-1-u9012063@gmail.com>
 <20210820022903.50d75750@sovereign>
In-Reply-To: <20210820022903.50d75750@sovereign>
From: William Tu <u9012063@gmail.com>
Date: Mon, 23 Aug 2021 05:34:44 -0700
Message-ID: <CALDO+Sb0b5SSGj1qaRaQcCgu3TRRnDipR7ODjdGHn9hfyxZyhw@mail.gmail.com>
To: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Cc: dpdk-dev <dev@dpdk.org>, Nick Connolly <nick.connolly@mayadata.io>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [dpdk-dev] [PATCH v7] eal: remove sys/queue.h from public
 headers.
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Dmitry,
Thanks for your feedback.

On Thu, Aug 19, 2021 at 4:29 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> 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.

Yes, it got lost. I will fix the documentation, and the
RTE_TAILQ_FOREACH_SAFE at rte_tailq.h.

>
> > +#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.

OK, I will remove all the inline comments.

Thanks,
William

William