Re: [PATCH] net/failsafe: safer subdev iterator

["Gaëtan Rivet" <gaetan.rivet@6wind.com>]

On Thu, Aug 17, 2017 at 05:39:29PM -0700, Stephen Hemminger wrote:
> On Thu, 17 Aug 2017 17:52:09 +0200
> Gaetan Rivet <gaetan.rivet@6wind.com> 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 <gaetan.rivet@6wind.com>
> > ---
> >  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