Re: [PATCH 8/8] spapr/drc: Clean up local variable shadowing in prop_get_fdt()

[Harsh Prateek Bora]

On 9/29/23 11:37, Cédric Le Goater wrote:
> On 9/29/23 07:39, Markus Armbruster wrote:
> > Harsh Prateek Bora <harsh.prateek.bora@gmail.com> writes:
> >
> > > On Tue, 19 Sept, 2023, 5:39 pm Cédric Le Goater, <clg@kaod.org> wrote:
> > >
> > > > On 9/19/23 10:48, Harsh Prateek Bora wrote:
> > > > > On 9/18/23 20:28, Cédric Le Goater wrote:
> > > > > > Rename 'name' variable to avoid this warning :
> > > > > >
> > > > > >     ../hw/ppc/spapr_drc.c: In function ‘prop_get_fdt’:
> > > > > >     ../hw/ppc/spapr_drc.c:344:21: warning: declaration of ‘name’ 
> > > > > > shadows
> > > > a parameter [-Wshadow=compatible-local]
> > > > > >       344 |         const char *name = NULL;
> > > > > >           |                     ^~~~
> > > > > >     ../hw/ppc/spapr_drc.c:325:63: note: shadowed declaration is here
> > > > > >       325 | static void prop_get_fdt(Object *obj, Visitor *v, 
> > > > > > const char
> > > > *name,
> > > > > >           |
> > > > ~~~~~~~~~~~~^~~~
> > > > > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > > > > > ---
> > > > > >    hw/ppc/spapr_drc.c | 10 +++++-----
> > > > > >    1 file changed, 5 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > > > > > index 843e318312d3..2b99d3b4b1a6 100644
> > > > > > --- a/hw/ppc/spapr_drc.c
> > > > > > +++ b/hw/ppc/spapr_drc.c
> > > > > > @@ -341,7 +341,7 @@ static void prop_get_fdt(Object *obj, Visitor *v,
> > > > const char *name,
> > > > > >        fdt_depth = 0;
> > > > > >        do {
> > > > > > -        const char *name = NULL;
> > > > > > +        const char *dt_name = NULL;
> > > > >
> > > > > I guess you wanted to use the input arg "name" here without
> > > > re-declaration.
> > > >
> > > > I don't understand. I don't want to use the input arg "name" here.
> > > > It seems useless in this case.
> > >
> > > Yeh, I realize now. This patch can actually remove the unused arg 
> > > "name" as
> > > well?
> >
> > Cédric?
> >
> > Lose ends like this one make me reluctant to queue a series, even when
> > they look minor to me.
>
> Unfortunately, we can not remove the unused arg "name" from the prototype.
> The routine is a ObjectPropertyAccessor argument of object_property_add().

Hmm, I see ..
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

> Thanks,
>
> C.
>
>
> > > > C.
> > > >
> > > > > I do not see "name" being used elsewhere in this routine.
> > > > >
> > > > > regards,
> > > > > Harsh
> > > > > >            const struct fdt_property *prop = NULL;
> > > > > >            int prop_len = 0, name_len = 0;
> > > > > >            uint32_t tag;
> > > > > > @@ -351,8 +351,8 @@ static void prop_get_fdt(Object *obj, Visitor *v,
> > > > const char *name,
> > > > > >            switch (tag) {
> > > > > >            case FDT_BEGIN_NODE:
> > > > > >                fdt_depth++;
> > > > > > -            name = fdt_get_name(fdt, fdt_offset, &name_len);
> > > > > > -            if (!visit_start_struct(v, name, NULL, 0, errp)) {
> > > > > > +            dt_name = fdt_get_name(fdt, fdt_offset, &name_len);
> > > > > > +            if (!visit_start_struct(v, dt_name, NULL, 0, errp)) {
> > > > > >                    return;
> > > > > >                }
> > > > > >                break;
> > > > > > @@ -369,8 +369,8 @@ static void prop_get_fdt(Object *obj, Visitor *v,
> > > > const char *name,
> > > > > >            case FDT_PROP: {
> > > > > >                int i;
> > > > > >                prop = fdt_get_property_by_offset(fdt, fdt_offset,
> > > > &prop_len);
> > > > > > -            name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
> > > > > > -            if (!visit_start_list(v, name, NULL, 0, errp)) {
> > > > > > +            dt_name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff));
> > > > > > +            if (!visit_start_list(v, dt_name, NULL, 0, errp)) {
> > > > > >                    return;
> > > > > >                }
> > > > > >                for (i = 0; i < prop_len; i++) {