From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; Mon, 23 Aug 2021 14:35:21 +0200 (CEST) Received: by mail-lj1-f176.google.com with SMTP id s3so31190200ljp.11 for ; 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 Date: Mon, 23 Aug 2021 05:34:44 -0700 Message-ID: To: Dmitry Kozlyuk Cc: dpdk-dev , Nick Connolly 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Dmitry, Thanks for your feedback. On Thu, Aug 19, 2021 at 4:29 PM Dmitry Kozlyuk 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 > > +#include > > + > > +/* 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 > > #include > > +#include > > > > #include > > #include > > 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