| Message ID | 20180816055756.1011374-5-amitay@ozlabs.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/5] libpdbg: Remove unused variable | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | master/apply_patch Successfully applied |
| snowpatch_ozlabs/build-multiarch | success | Test build-multiarch on branch master |
Hi Amitay, What is this needed for? In the targetting model we have we are trying to avoid relying on overly specific parent/child relationships as they can be brittle, hence why callers need to specify which specific type/class of parent they are looking for. There's not much that could be done with an arbitrary parent anyway. Although I agree an `assert(class)` would probably assist debug. - Alistair On Thursday, 16 August 2018 3:57:56 PM AEST Amitay Isaacs wrote: > This avoids the segfault if class is NULL. Also, this api can be used > to traverse the tree by explicitly setting class=NULL. > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > --- > libpdbg/libpdbg.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c > index 522bec9..810e045 100644 > --- a/libpdbg/libpdbg.c > +++ b/libpdbg/libpdbg.c > @@ -89,6 +89,9 @@ struct pdbg_target *pdbg_target_parent(const char *class, struct pdbg_target *ta > { > struct pdbg_target *parent; > > + if (!class) > + return target->parent; > + > for (parent = target->parent; parent && parent->parent; parent = parent->parent) { > if (!strcmp(class, pdbg_target_class_name(parent))) > return parent; >
On Fri, 2018-09-07 at 13:23 +1000, Alistair Popple wrote: > Hi Amitay, > > What is this needed for? In the targetting model we have we are > trying to avoid > relying on overly specific parent/child relationships as they can be > brittle, > hence why callers need to specify which specific type/class of parent > they are > looking for. There's not much that could be done with an arbitrary > parent > anyway. Well we need some mechanisms for traverseing the tree. I am re-using this function to also do that. May be we need separate api when we are just traversing the tree and when we are searching for targets based on class. I needed this functionality in testing to go up and down the device tree from a node. > > Although I agree an `assert(class)` would probably assist debug. > > - Alistair > > On Thursday, 16 August 2018 3:57:56 PM AEST Amitay Isaacs wrote: > > This avoids the segfault if class is NULL. Also, this api can be > > used > > to traverse the tree by explicitly setting class=NULL. > > > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > > --- > > libpdbg/libpdbg.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c > > index 522bec9..810e045 100644 > > --- a/libpdbg/libpdbg.c > > +++ b/libpdbg/libpdbg.c > > @@ -89,6 +89,9 @@ struct pdbg_target *pdbg_target_parent(const char > > *class, struct pdbg_target *ta > > { > > struct pdbg_target *parent; > > > > + if (!class) > > + return target->parent; > > + > > for (parent = target->parent; parent && parent->parent; parent > > = parent->parent) { > > if (!strcmp(class, pdbg_target_class_name(parent))) > > return parent; > > > > Amitay.
On Friday, 7 September 2018 2:03:45 PM AEST Amitay Isaacs wrote: > On Fri, 2018-09-07 at 13:23 +1000, Alistair Popple wrote: > > Hi Amitay, > > > > What is this needed for? In the targetting model we have we are > > trying to avoid > > relying on overly specific parent/child relationships as they can be > > brittle, > > hence why callers need to specify which specific type/class of parent > > they are > > looking for. There's not much that could be done with an arbitrary > > parent > > anyway. > > Well we need some mechanisms for traverseing the tree. I am re-using > this function to also do that. May be we need separate api when we are > just traversing the tree and when we are searching for targets based on > class. > > I needed this functionality in testing to go up and down the device > tree from a node. You could just use pdbg_for_each_child_target() to traverse down the tree. Although the existance of this functionality negates my previous argument about relying on searching for specific classes/types so I'll merge this (which is just the reverse of what we already have) as well. Thanks! - Alistair > > > > Although I agree an `assert(class)` would probably assist debug. > > > > - Alistair > > > > On Thursday, 16 August 2018 3:57:56 PM AEST Amitay Isaacs wrote: > > > This avoids the segfault if class is NULL. Also, this api can be > > > used > > > to traverse the tree by explicitly setting class=NULL. > > > > > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > > > --- > > > libpdbg/libpdbg.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c > > > index 522bec9..810e045 100644 > > > --- a/libpdbg/libpdbg.c > > > +++ b/libpdbg/libpdbg.c > > > @@ -89,6 +89,9 @@ struct pdbg_target *pdbg_target_parent(const char > > > *class, struct pdbg_target *ta > > > { > > > struct pdbg_target *parent; > > > > > > + if (!class) > > > + return target->parent; > > > + > > > for (parent = target->parent; parent && parent->parent; parent > > > = parent->parent) { > > > if (!strcmp(class, pdbg_target_class_name(parent))) > > > return parent; > > > > > > > > > Amitay. >
diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c index 522bec9..810e045 100644 --- a/libpdbg/libpdbg.c +++ b/libpdbg/libpdbg.c @@ -89,6 +89,9 @@ struct pdbg_target *pdbg_target_parent(const char *class, struct pdbg_target *ta { struct pdbg_target *parent; + if (!class) + return target->parent; + for (parent = target->parent; parent && parent->parent; parent = parent->parent) { if (!strcmp(class, pdbg_target_class_name(parent))) return parent;
This avoids the segfault if class is NULL. Also, this api can be used to traverse the tree by explicitly setting class=NULL. Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> --- libpdbg/libpdbg.c | 3 +++ 1 file changed, 3 insertions(+)