From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet Subject: Re: [PATCH] net/failsafe: safer subdev iterator Date: Fri, 18 Aug 2017 09:43:01 +0200 Message-ID: <20170818074301.GR8124@bidouze.vm.6wind.com> References: <1502985129-7461-1-git-send-email-gaetan.rivet@6wind.com> <20170817173929.4f8951c3@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org To: Stephen Hemminger Return-path: Received: from mail-wr0-f172.google.com (mail-wr0-f172.google.com [209.85.128.172]) by dpdk.org (Postfix) with ESMTP id 7C8FF7CC9 for ; Fri, 18 Aug 2017 09:43:12 +0200 (CEST) Received: by mail-wr0-f172.google.com with SMTP id z91so54633625wrc.4 for ; Fri, 18 Aug 2017 00:43:12 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20170817173929.4f8951c3@xeon-e3> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Aug 17, 2017 at 05:39:29PM -0700, Stephen Hemminger wrote: > On Thu, 17 Aug 2017 17:52:09 +0200 > Gaetan Rivet wrote: > > > The sub_device iterator macro should follow the general gist of the > > tailq API for an easier understanding and safer use. > > > > Once the loop has finished, the iterator should be set to NULL. > > If no sub_device was iterated upon, the iterator should still be NULL. > > > > Signed-off-by: Gaetan Rivet > > --- > > drivers/net/failsafe/failsafe_private.h | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h > > index 0361cf4..6fa8de5 100644 > > --- a/drivers/net/failsafe/failsafe_private.h > > +++ b/drivers/net/failsafe/failsafe_private.h > > @@ -222,6 +222,7 @@ extern int mac_from_arg; > > */ > > #define FOREACH_SUBDEV_STATE(s, i, dev, state) \ > > for (i = fs_find_next((dev), 0, state); \ > > + ((s = NULL) == NULL) && \ > I assume you are trying to do assignment inside an expression and just expect that > to always to be true and go onto next part of assignment. This is a convulted > way to put code in the loop iterator. > > But this macro is way too complex. Please break it up into inline functions or > figure out how to simplify the logic better. I think you are right, thanks for the wake-up call. -- Gaëtan Rivet 6WIND