[sheepdog] [PATCH v2 01/11] rbtree: make rb_for_each_entry and rb_for_each more generic

[MORITA Kazutaka]

At Thu, 19 Sep 2013 15:06:44 +0800,
Liu Yuan wrote:
> 
> On Thu, Sep 19, 2013 at 02:35:35AM +0900, MORITA Kazutaka wrote:
> > At Sat, 14 Sep 2013 18:34:21 +0800,
> > Liu Yuan wrote:
> > > 
> > > This allows us to call rb_xxx helpers inside the rb_for_xx helpers.
> > > 
> > > Signed-off-by: Liu Yuan <namei.unix at gmail.com>
> > > ---
> > >  include/rbtree.h |   12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/include/rbtree.h b/include/rbtree.h
> > > index 92c5389..07b6fe6 100644
> > > --- a/include/rbtree.h
> > > +++ b/include/rbtree.h
> > > @@ -164,16 +164,16 @@ static inline void rb_link_node(struct rb_node *node, struct rb_node *parent,
> > >  /* Iterate over a rbtree safe against removal of rbnode */
> > >  #define rb_for_each(pos, root)					\
> > >  	pos = rb_first(root);					\
> > > -	for (struct rb_node *__n = rb_next(pos);		\
> > > -	     pos && ({ __n = rb_next(pos); 1; });		\
> > > -	     pos = __n)
> > > +	for (struct rb_node *__n1 = rb_next(pos);		\
> > > +	     pos && ({ __n1 = rb_next(pos); 1; });		\
> > > +	     pos = __n1)
> > >  
> > >  /* Iterate over a rbtree of given type safe against removal of rbnode */
> > >  #define rb_for_each_entry(pos, root, member)				\
> > > -	for (struct rb_node *__pos = rb_first(root), *__n;		\
> > > -	     __pos && ({ __n = rb_next(__pos); 1; }) &&			\
> > > +	for (struct rb_node *__pos = rb_first(root), *__n2;		\
> > > +	     __pos && ({ __n2 = rb_next(__pos); 1; }) &&		\
> > >  		     ({ pos =  rb_entry(__pos, typeof(*pos), member); 1; }); \
> > > -	     __pos = __n)
> > > +	     __pos = __n2)
> > 
> > Then, we have to introduce __n3, __n4, ... to call macros inside
> > rb_for_each_entry()?  I think we should use better name convention.
> > E.g. adding a prefix like "__rb_for_each_entry_n" is unlikely to cause
> > collision though it's a bit too long.
> > 
> > I'd suggest adding a line number to the variable.  The below macro
> > generates a dynamic variable name with a line number.
> > 
> > #define __LOCAL(var, line) __ ## var ## line
> > #define _LOCAL(var, line) __LOCAL(var, line)
> > #define LOCAL(var) _LOCAL(var, __LINE__)
> > 
> > Then rb_for_each_entry() would be as follows.
> > 
> >   for (struct rb_node *LOCAL(pos) = rb_first(root), *LOCAL(n);    \
> >        LOCAL(pos) && ({ LOCAL(n) = rb_next(LOCAL(pos)); 1; }) &&  \
> >                ({ pos =  rb_entry(LOCAL(pos), typeof(*pos), member); 1; }); \
> >        LOCAL(pos) = LOCAL(n))
> > 
> > This also allows us to use rb_for_each_entry() inside rb_for_each_entry().
> 
> In file included from sockfd_cache.c:36:0:
> ../include/sheep.h: In function ‘nodes_to_vnodes’:
> ../include/sheep.h:226:1: error: redeclaration of ‘__n226’ with no linkage
> ../include/sheep.h:226:1: note: previous definition of ‘__n226’ was here
> ../include/sheep.h: In function ‘nodes_to_buffer’:
> ../include/sheep.h:246:1: error: redeclaration of ‘__n246’ with no linkage
> ../include/sheep.h:246:1: note: previous definition of ‘__n246’ was here
> sockfd_cache.c: In function ‘sockfd_cache_add_group’:
> sockfd_cache.c:213:1: error: redeclaration of ‘__n213’ with no linkage
> sockfd_cache.c:213:1: note: previous definition of ‘__n213’ was here
> 
> I can't pass compile with this trick.

Well, I have no idea why the problem happens.  I think of refactoring
rb_tree macro and fixing the variable name issue, so please ignore my
comment and go ahead with your code.

Thanks,

Kazutaka