| Message ID | 20190806013723.4047-9-alistair@popple.id.au |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Split backends from system description | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (d0a0d919cccbd69631aaef0d37f1dba8d53e86e0) |
| snowpatch_ozlabs/build-multiarch | success | Test build-multiarch on branch master |
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.
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. >
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.
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. >
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;
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(-)