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

[Markus Armbruster]

Harsh Prateek Bora <harsh.prateek.bora@gmail.com> writes:

> On Tue, 19 Sept, 2023, 5:33 pm Cédric Le Goater, <clg@kaod.org> wrote:
>
>> On 9/19/23 10:29, Harsh Prateek Bora wrote:
>> >
>> >
>> > On 9/18/23 20:28, Cédric Le Goater wrote:
>> >> Remove extra 'drc_index' variable to avoid this warning :
>> >>
>> >>    ../hw/ppc/spapr_drc.c: In function ‘rtas_ibm_configure_connector’:
>> >>    ../hw/ppc/spapr_drc.c:1240:26: warning: declaration of ‘drc_index’
>> shadows a previous local [-Wshadow=compatible-local]
>> >>     1240 |                 uint32_t drc_index = spapr_drc_index(drc);
>> >>          |                          ^~~~~~~~~
>> >>    ../hw/ppc/spapr_drc.c:1155:14: note: shadowed declaration is here
>> >>     1155 |     uint32_t drc_index;
>> >>          |              ^~~~~~~~~
>> >>
>> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> >> ---
>> >>   hw/ppc/spapr_drc.c | 2 --
>> >>   1 file changed, 2 deletions(-)
>> >>
>> >> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
>> >> index b5c400a94d1c..843e318312d3 100644
>> >> --- a/hw/ppc/spapr_drc.c
>> >> +++ b/hw/ppc/spapr_drc.c
>> >> @@ -1237,8 +1237,6 @@ static void
>> rtas_ibm_configure_connector(PowerPCCPU *cpu,
>> >>           case FDT_END_NODE:
>> >>               drc->ccs_depth--;
>> >>               if (drc->ccs_depth == 0) {
>> >> -                uint32_t drc_index = spapr_drc_index(drc);
>> >> -
>> > I guess you only wanted to remove re-declaration part. Assigning the
>>
>> value returned by this function doesnt seem to happen before.
>>
>> drc_index is assigned at the top of this large routine with :
>>
>>      drc_index = rtas_ld(wa_addr, 0);
>>      drc = spapr_drc_by_index(drc_index);
>>
>>
>> So, the extra local variable 'drc_index' is simply redundant because
>> there are no reason for it to change. The drc object is the same AFAICT.
>> Correct ? I should have explained that better in the commit log.
>>
>
> Okay, since both routines were implemented differently, I wasn't sure about
> the impact of reassignment. Better commit log is always welcome.

Do you expect a respin?  If not, would you like to give your R-by?