[RFC,08/12] libpdbg: Change get_target_class() to take a pdbg_target
diff mbox series

Message ID 20190806013723.4047-9-alistair@popple.id.au
State New
Headers show
Series
  • Split backends from system description
Related show

Checks

Context Check Description
snowpatch_ozlabs/build-multiarch success Test build-multiarch on branch master
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (d0a0d919cccbd69631aaef0d37f1dba8d53e86e0)

Commit Message

Alistair Popple Aug. 6, 2019, 1:37 a.m. UTC
No behavioural change.

Signed-off-by: Alistair Popple <alistair@popple.id.au>
---
 libpdbg/device.c | 2 +-
 libpdbg/target.c | 8 ++++----
 libpdbg/target.h | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

Comments

Amitay Isaacs Aug. 20, 2019, 4:04 a.m. UTC | #1
Reviewed-by: Amitay Isaacs <amitay@ozlabs.org>

with one fix below.

On Tue, 2019-08-06 at 11:37 +1000, Alistair Popple wrote:
> No behavioural change.
> 
> Signed-off-by: Alistair Popple <alistair@popple.id.au>
> ---
>  libpdbg/device.c | 2 +-
>  libpdbg/target.c | 8 ++++----
>  libpdbg/target.h | 2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libpdbg/device.c b/libpdbg/device.c
> index 21ec3a3..f6d27db 100644
> --- a/libpdbg/device.c
> +++ b/libpdbg/device.c
> @@ -110,7 +110,7 @@ static struct pdbg_target *pdbg_target_new(const
> void *fdt, int node_offset)
>  	 * guaranteed to be the struct pdbg_target (see the comment
>  	 * above DECLARE_HW_UNIT). */
>  	memcpy(target, hw_info->hw_unit, size);
> -	target_class = get_target_class(target->class);
> +	target_class = get_target_class(target);
>  	list_add_tail(&target_class->targets, &target->class_link);
>  
>  	return target;
> diff --git a/libpdbg/target.c b/libpdbg/target.c
> index e822d70..73ad98f 100644
> --- a/libpdbg/target.c
> +++ b/libpdbg/target.c
> @@ -334,18 +334,18 @@ struct pdbg_target_class
> *require_target_class(const char *name)
>  }
>  
>  /* Returns the existing class or allocates space for a new one */
> -struct pdbg_target_class *get_target_class(const char *name)
> +struct pdbg_target_class *get_target_class(struct pdbg_target
> *target)
>  {
>  	struct pdbg_target_class *target_class;
>  
> -	if ((target_class = find_target_class(name)))
> +	if ((target_class = find_target_class(target->class)))

This should be:   ... = find_target_class(target)

>  		return target_class;
>  
>  	/* Need to allocate a new class */
> -	PR_DEBUG("Allocating %s target class\n", name);
> +	PR_DEBUG("Allocating %s target class\n", target->class);
>  	target_class = calloc(1, sizeof(*target_class));
>  	assert(target_class);
> -	target_class->name = strdup(name);
> +	target_class->name = strdup(target->class);
>  	list_head_init(&target_class->targets);
>  	list_add_tail(&target_classes, &target_class->class_head_link);
>  	return target_class;
> diff --git a/libpdbg/target.h b/libpdbg/target.h
> index 2c76bf9..44cbcde 100644
> --- a/libpdbg/target.h
> +++ b/libpdbg/target.h
> @@ -56,7 +56,7 @@ struct pdbg_target {
>  struct pdbg_target *require_target_parent(struct pdbg_target
> *target);
>  struct pdbg_target_class *find_target_class(const char *name);
>  struct pdbg_target_class *require_target_class(const char *name);
> -struct pdbg_target_class *get_target_class(const char *name);
> +struct pdbg_target_class *get_target_class(struct pdbg_target
> *target);
>  bool pdbg_target_is_class(struct pdbg_target *target, const char
> *class);
>  
>  extern struct list_head empty_list;
> -- 
> 2.20.1
> 

Amitay.
Alistair Popple Aug. 20, 2019, 6:14 a.m. UTC | #2
On Tuesday, 20 August 2019 2:04:40 PM AEST Amitay Isaacs wrote:
> Reviewed-by: Amitay Isaacs <amitay@ozlabs.org>
> 
> with one fix below.
> 
> On Tue, 2019-08-06 at 11:37 +1000, Alistair Popple wrote:
> > No behavioural change.
> > 
> > Signed-off-by: Alistair Popple <alistair@popple.id.au>
> > ---
> >  libpdbg/device.c | 2 +-
> >  libpdbg/target.c | 8 ++++----
> >  libpdbg/target.h | 2 +-
> >  3 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/libpdbg/device.c b/libpdbg/device.c
> > index 21ec3a3..f6d27db 100644
> > --- a/libpdbg/device.c
> > +++ b/libpdbg/device.c
> > @@ -110,7 +110,7 @@ static struct pdbg_target *pdbg_target_new(const
> > void *fdt, int node_offset)
> >  	 * guaranteed to be the struct pdbg_target (see the comment
> >  	 * above DECLARE_HW_UNIT). */
> >  	memcpy(target, hw_info->hw_unit, size);
> > -	target_class = get_target_class(target->class);
> > +	target_class = get_target_class(target);
> >  	list_add_tail(&target_class->targets, &target->class_link);
> >  
> >  	return target;
> > diff --git a/libpdbg/target.c b/libpdbg/target.c
> > index e822d70..73ad98f 100644
> > --- a/libpdbg/target.c
> > +++ b/libpdbg/target.c
> > @@ -334,18 +334,18 @@ struct pdbg_target_class
> > *require_target_class(const char *name)
> >  }
> >  
> >  /* Returns the existing class or allocates space for a new one */
> > -struct pdbg_target_class *get_target_class(const char *name)
> > +struct pdbg_target_class *get_target_class(struct pdbg_target
> > *target)
> >  {
> >  	struct pdbg_target_class *target_class;
> >  
> > -	if ((target_class = find_target_class(name)))
> > +	if ((target_class = find_target_class(target->class)))
> 
> This should be:   ... = find_target_class(target)

No, I think the original is correct. Or are you suggesting the we should 
change find_target_class() to take a struct pdbg_target instead of the class 
name?

- Alistair

> >  		return target_class;
> >  
> >  	/* Need to allocate a new class */
> > -	PR_DEBUG("Allocating %s target class\n", name);
> > +	PR_DEBUG("Allocating %s target class\n", target->class);
> >  	target_class = calloc(1, sizeof(*target_class));
> >  	assert(target_class);
> > -	target_class->name = strdup(name);
> > +	target_class->name = strdup(target->class);
> >  	list_head_init(&target_class->targets);
> >  	list_add_tail(&target_classes, &target_class->class_head_link);
> >  	return target_class;
> > diff --git a/libpdbg/target.h b/libpdbg/target.h
> > index 2c76bf9..44cbcde 100644
> > --- a/libpdbg/target.h
> > +++ b/libpdbg/target.h
> > @@ -56,7 +56,7 @@ struct pdbg_target {
> >  struct pdbg_target *require_target_parent(struct pdbg_target
> > *target);
> >  struct pdbg_target_class *find_target_class(const char *name);
> >  struct pdbg_target_class *require_target_class(const char *name);
> > -struct pdbg_target_class *get_target_class(const char *name);
> > +struct pdbg_target_class *get_target_class(struct pdbg_target
> > *target);
> >  bool pdbg_target_is_class(struct pdbg_target *target, const char
> > *class);
> >  
> >  extern struct list_head empty_list;
> 
> Amitay.
>
Amitay Isaacs Aug. 20, 2019, 6:37 a.m. UTC | #3
On Tue, 2019-08-20 at 16:14 +1000, Alistair Popple wrote:
> On Tuesday, 20 August 2019 2:04:40 PM AEST Amitay Isaacs wrote:
> > Reviewed-by: Amitay Isaacs <amitay@ozlabs.org>
> > 
> > with one fix below.
> > 
> > On Tue, 2019-08-06 at 11:37 +1000, Alistair Popple wrote:
> > > No behavioural change.
> > > 
> > > Signed-off-by: Alistair Popple <alistair@popple.id.au>
> > > ---
> > >  libpdbg/device.c | 2 +-
> > >  libpdbg/target.c | 8 ++++----
> > >  libpdbg/target.h | 2 +-
> > >  3 files changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/libpdbg/device.c b/libpdbg/device.c
> > > index 21ec3a3..f6d27db 100644
> > > --- a/libpdbg/device.c
> > > +++ b/libpdbg/device.c
> > > @@ -110,7 +110,7 @@ static struct pdbg_target
> > > *pdbg_target_new(const
> > > void *fdt, int node_offset)
> > >  	 * guaranteed to be the struct pdbg_target (see the comment
> > >  	 * above DECLARE_HW_UNIT). */
> > >  	memcpy(target, hw_info->hw_unit, size);
> > > -	target_class = get_target_class(target->class);
> > > +	target_class = get_target_class(target);
> > >  	list_add_tail(&target_class->targets, &target->class_link);
> > >  
> > >  	return target;
> > > diff --git a/libpdbg/target.c b/libpdbg/target.c
> > > index e822d70..73ad98f 100644
> > > --- a/libpdbg/target.c
> > > +++ b/libpdbg/target.c
> > > @@ -334,18 +334,18 @@ struct pdbg_target_class
> > > *require_target_class(const char *name)
> > >  }
> > >  
> > >  /* Returns the existing class or allocates space for a new one
> > > */
> > > -struct pdbg_target_class *get_target_class(const char *name)
> > > +struct pdbg_target_class *get_target_class(struct pdbg_target
> > > *target)
> > >  {
> > >  	struct pdbg_target_class *target_class;
> > >  
> > > -	if ((target_class = find_target_class(name)))
> > > +	if ((target_class = find_target_class(target->class)))
> > 
> > This should be:   ... = find_target_class(target)
> 
> No, I think the original is correct. Or are you suggesting the we
> should 
> change find_target_class() to take a struct pdbg_target instead of
> the class 
> name?

Damn your naming! :-)

I got confused between get_target_class() and find_target_class().  

Amitay.

> 
> - Alistair
> 
> > >  		return target_class;
> > >  
> > >  	/* Need to allocate a new class */
> > > -	PR_DEBUG("Allocating %s target class\n", name);
> > > +	PR_DEBUG("Allocating %s target class\n", target->class);
> > >  	target_class = calloc(1, sizeof(*target_class));
> > >  	assert(target_class);
> > > -	target_class->name = strdup(name);
> > > +	target_class->name = strdup(target->class);
> > >  	list_head_init(&target_class->targets);
> > >  	list_add_tail(&target_classes, &target_class->class_head_link);
> > >  	return target_class;
> > > diff --git a/libpdbg/target.h b/libpdbg/target.h
> > > index 2c76bf9..44cbcde 100644
> > > --- a/libpdbg/target.h
> > > +++ b/libpdbg/target.h
> > > @@ -56,7 +56,7 @@ struct pdbg_target {
> > >  struct pdbg_target *require_target_parent(struct pdbg_target
> > > *target);
> > >  struct pdbg_target_class *find_target_class(const char *name);
> > >  struct pdbg_target_class *require_target_class(const char
> > > *name);
> > > -struct pdbg_target_class *get_target_class(const char *name);
> > > +struct pdbg_target_class *get_target_class(struct pdbg_target
> > > *target);
> > >  bool pdbg_target_is_class(struct pdbg_target *target, const char
> > > *class);
> > >  
> > >  extern struct list_head empty_list;
> > 
> > Amitay.
> > 
> 
> 
> 

Amitay.
Alistair Popple Aug. 20, 2019, 6:45 a.m. UTC | #4
On Tuesday, 20 August 2019 4:37:27 PM AEST Amitay Isaacs wrote:
> On Tue, 2019-08-20 at 16:14 +1000, Alistair Popple wrote:
> > On Tuesday, 20 August 2019 2:04:40 PM AEST Amitay Isaacs wrote:
> > > Reviewed-by: Amitay Isaacs <amitay@ozlabs.org>
> > > 
> > > with one fix below.
> > > 
> > > On Tue, 2019-08-06 at 11:37 +1000, Alistair Popple wrote:
> > > > No behavioural change.
> > > > 
> > > > Signed-off-by: Alistair Popple <alistair@popple.id.au>
> > > > ---
> > > >  libpdbg/device.c | 2 +-
> > > >  libpdbg/target.c | 8 ++++----
> > > >  libpdbg/target.h | 2 +-
> > > >  3 files changed, 6 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/libpdbg/device.c b/libpdbg/device.c
> > > > index 21ec3a3..f6d27db 100644
> > > > --- a/libpdbg/device.c
> > > > +++ b/libpdbg/device.c
> > > > @@ -110,7 +110,7 @@ static struct pdbg_target
> > > > *pdbg_target_new(const
> > > > void *fdt, int node_offset)
> > > >  	 * guaranteed to be the struct pdbg_target (see the comment
> > > >  	 * above DECLARE_HW_UNIT). */
> > > >  	memcpy(target, hw_info->hw_unit, size);
> > > > -	target_class = get_target_class(target->class);
> > > > +	target_class = get_target_class(target);
> > > >  	list_add_tail(&target_class->targets, &target->class_link);
> > > >  
> > > >  	return target;
> > > > diff --git a/libpdbg/target.c b/libpdbg/target.c
> > > > index e822d70..73ad98f 100644
> > > > --- a/libpdbg/target.c
> > > > +++ b/libpdbg/target.c
> > > > @@ -334,18 +334,18 @@ struct pdbg_target_class
> > > > *require_target_class(const char *name)
> > > >  }
> > > >  
> > > >  /* Returns the existing class or allocates space for a new one
> > > > */
> > > > -struct pdbg_target_class *get_target_class(const char *name)
> > > > +struct pdbg_target_class *get_target_class(struct pdbg_target
> > > > *target)
> > > >  {
> > > >  	struct pdbg_target_class *target_class;
> > > >  
> > > > -	if ((target_class = find_target_class(name)))
> > > > +	if ((target_class = find_target_class(target->class)))
> > > 
> > > This should be:   ... = find_target_class(target)
> > 
> > No, I think the original is correct. Or are you suggesting the we
> > should 
> > change find_target_class() to take a struct pdbg_target instead of
> > the class 
> > name?
> 
> Damn your naming! :-)

Hehe. Terrible naming I agree. find_target_class() really should just be a 
static helper function but we use it once in libpdbg.c

- Alistair

> I got confused between get_target_class() and find_target_class().  
> 
> Amitay.
> 
> > 
> > - Alistair
> > 
> > > >  		return target_class;
> > > >  
> > > >  	/* Need to allocate a new class */
> > > > -	PR_DEBUG("Allocating %s target class\n", name);
> > > > +	PR_DEBUG("Allocating %s target class\n", target->class);
> > > >  	target_class = calloc(1, sizeof(*target_class));
> > > >  	assert(target_class);
> > > > -	target_class->name = strdup(name);
> > > > +	target_class->name = strdup(target->class);
> > > >  	list_head_init(&target_class->targets);
> > > >  	list_add_tail(&target_classes, &target_class->class_head_link);
> > > >  	return target_class;
> > > > diff --git a/libpdbg/target.h b/libpdbg/target.h
> > > > index 2c76bf9..44cbcde 100644
> > > > --- a/libpdbg/target.h
> > > > +++ b/libpdbg/target.h
> > > > @@ -56,7 +56,7 @@ struct pdbg_target {
> > > >  struct pdbg_target *require_target_parent(struct pdbg_target
> > > > *target);
> > > >  struct pdbg_target_class *find_target_class(const char *name);
> > > >  struct pdbg_target_class *require_target_class(const char
> > > > *name);
> > > > -struct pdbg_target_class *get_target_class(const char *name);
> > > > +struct pdbg_target_class *get_target_class(struct pdbg_target
> > > > *target);
> > > >  bool pdbg_target_is_class(struct pdbg_target *target, const char
> > > > *class);
> > > >  
> > > >  extern struct list_head empty_list;
> > > 
> > > Amitay.
> > > 
> > 
> > 
> > 
> 
> Amitay.
>

Patch
diff mbox series

diff --git a/libpdbg/device.c b/libpdbg/device.c
index 21ec3a3..f6d27db 100644
--- a/libpdbg/device.c
+++ b/libpdbg/device.c
@@ -110,7 +110,7 @@  static struct pdbg_target *pdbg_target_new(const void *fdt, int node_offset)
 	 * guaranteed to be the struct pdbg_target (see the comment
 	 * above DECLARE_HW_UNIT). */
 	memcpy(target, hw_info->hw_unit, size);
-	target_class = get_target_class(target->class);
+	target_class = get_target_class(target);
 	list_add_tail(&target_class->targets, &target->class_link);
 
 	return target;
diff --git a/libpdbg/target.c b/libpdbg/target.c
index e822d70..73ad98f 100644
--- a/libpdbg/target.c
+++ b/libpdbg/target.c
@@ -334,18 +334,18 @@  struct pdbg_target_class *require_target_class(const char *name)
 }
 
 /* Returns the existing class or allocates space for a new one */
-struct pdbg_target_class *get_target_class(const char *name)
+struct pdbg_target_class *get_target_class(struct pdbg_target *target)
 {
 	struct pdbg_target_class *target_class;
 
-	if ((target_class = find_target_class(name)))
+	if ((target_class = find_target_class(target->class)))
 		return target_class;
 
 	/* Need to allocate a new class */
-	PR_DEBUG("Allocating %s target class\n", name);
+	PR_DEBUG("Allocating %s target class\n", target->class);
 	target_class = calloc(1, sizeof(*target_class));
 	assert(target_class);
-	target_class->name = strdup(name);
+	target_class->name = strdup(target->class);
 	list_head_init(&target_class->targets);
 	list_add_tail(&target_classes, &target_class->class_head_link);
 	return target_class;
diff --git a/libpdbg/target.h b/libpdbg/target.h
index 2c76bf9..44cbcde 100644
--- a/libpdbg/target.h
+++ b/libpdbg/target.h
@@ -56,7 +56,7 @@  struct pdbg_target {
 struct pdbg_target *require_target_parent(struct pdbg_target *target);
 struct pdbg_target_class *find_target_class(const char *name);
 struct pdbg_target_class *require_target_class(const char *name);
-struct pdbg_target_class *get_target_class(const char *name);
+struct pdbg_target_class *get_target_class(struct pdbg_target *target);
 bool pdbg_target_is_class(struct pdbg_target *target, const char *class);
 
 extern struct list_head empty_list;