[Pdbg] [PATCH 08/23] libpdbg: Child traversal should not include virtual nodes

[Amitay Isaacs]

On Thu, 2019-09-19 at 14:02 +1000, Alistair Popple wrote:
> > -struct pdbg_target *__pdbg_next_child_target(struct pdbg_target
> > *parent, 
> struct pdbg_target *last)
> > +static struct pdbg_target *pdbg_next_target_match(struct
> > pdbg_target *next, 
> bool virtual)
> 
> This is really two different functions selected by a flag, although I
> suppose in 
> the context it makes sense.
> 
> >  {
> > -	if (!parent || list_empty(&parent->children))
> > +	if (virtual) {
> > +		if (!next->vnode)
> > +			return next;
> > +		else
> > +			if (!next->compatible)
> > +				return next->vnode;
> > +	} else {
> > +		if (next->compatible)
> > +			return next;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +struct pdbg_target *__pdbg_next_child_target(struct pdbg_target
> > *parent, 
> struct pdbg_target *last, bool virtual)
> 
> However do we even need the virtual flag? I couldn't see anything
> that actually 
> calls __pdbg_next_child_target() with virtual == false, so perhaps we
> just 
> make traversing the system tree the only exported behaviour?

I am using virtual == false in the test code added later to confirm the
backend device tree traversal.

> 
> - Alistair
> 
> > +{
> > +	struct pdbg_target *next, *child = NULL;
> > +
> > +	if (!parent)
> >  		return NULL;
> >  
> > -	if (!last)
> > -		return list_top(&parent->children, struct pdbg_target,
> > list);
> > +	if (list_empty(&parent->children)) {
> > +		if (parent->vnode)
> > +			parent = parent->vnode;
> > +		else
> > +			return NULL;
> > +	}
> > +
> > +	if (!last) {
> > +		list_for_each(&parent->children, child, list)
> > +			if ((next = pdbg_next_target_match(child,
> > virtual)))
> > +				return next;
> > +
> > +		return NULL;
> > +	}
> > +
> > +	if (virtual && last->vnode && last->compatible)
> > +		last = last->vnode;
> >  
> > -	if (last->list.next == &parent->children.n)
> > +	if (last == list_tail(&parent->children, struct pdbg_target,
> > list))
> >  		return NULL;
> >  
> > -	return list_entry(last->list.next, struct pdbg_target, list);
> > +	child = last;
> > +	do {
> > +		child = list_entry(child->list.next, struct
> > pdbg_target, list);
> > +		if ((next = pdbg_next_target_match(child, virtual)))
> > +			return next;
> > +	} while (child != list_tail(&parent->children, struct
> > pdbg_target, list));
> > +
> > +	return NULL;
> >  }
> >  
> >  enum pdbg_target_status pdbg_target_status(struct pdbg_target
> > *target)
> > diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> > index caf8b85..df74e43 100644
> > --- a/libpdbg/libpdbg.h
> > +++ b/libpdbg/libpdbg.h
> > @@ -20,7 +20,7 @@ struct pdbg_target
> > *__pdbg_next_compatible_node(struct 
> pdbg_target *root,
> >                                                  struct pdbg_target
> > *prev,
> >                                                  const char
> > *compat);
> >  struct pdbg_target *__pdbg_next_target(const char *klass, struct 
> pdbg_target *parent, struct pdbg_target *last);
> > -struct pdbg_target *__pdbg_next_child_target(struct pdbg_target
> > *parent, 
> struct pdbg_target *last);
> > +struct pdbg_target *__pdbg_next_child_target(struct pdbg_target
> > *parent, 
> struct pdbg_target *last, bool virtual);
> >  
> >  /*
> >   * Each target has a status associated with it. This is what each
> > status 
> means:
> > @@ -71,9 +71,9 @@ enum pdbg_backend { PDBG_DEFAULT_BACKEND = 0, 
> PDBG_BACKEND_FSI, PDBG_BACKEND_I2C
> >  	     target = __pdbg_next_target(class, NULL, target))
> >  
> >  #define pdbg_for_each_child_target(parent, target)	      \
> > -	for (target = __pdbg_next_child_target(parent, NULL); \
> > +	for (target = __pdbg_next_child_target(parent, NULL, true); \
> >  	     target;					      \
> > -	     target = __pdbg_next_child_target(parent, target))
> > +	     target = __pdbg_next_child_target(parent, target, true))
> >  
> >  /* Return the first parent target of the given class, or NULL if
> > the given
> >   * target does not have a parent of the given class. */
> > 
> 
> 
> 

Amitay.
-- 

If a glass of wine pleases you, enjoy it for its own sake but do not
pour another glass from the same bottle; it cannot enhance the
experience.  Better to try a glass of a different type of wine on a
different day.