[U-Boot] [PATCH 7/7] GCC4.6: Squash warning in cmd_mem.c

Simon Glass
2011-09-26T19:01:16Z

Hi Marek,

On Mon, Sep 26, 2011 at 11:34 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> On Monday, September 26, 2011 08:29:22 PM Simon Glass wrote:
>> Hi Merek,
>>
>> On Mon, Sep 26, 2011 at 11:24 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> > On Monday, September 26, 2011 08:05:43 PM Simon Glass wrote:
>> >> Hi Marek,
>> >>
>> >> On Sun, Sep 25, 2011 at 5:26 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> >> > cmd_mem.c: In function ?do_mem_loop?:
>> >> > cmd_mem.c:474:25: warning: variable ?junk? set but not used
>> >> > [-Wunused-but-set-variable]
>> >> >
>> >> > The assigned variable can be removed because the pointers are volatile
>> >> > so accesses to their addresses are always generated.
>> >> >
>> >> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>> >> > ---
>> >> > ?common/cmd_mem.c | ? ?8 ++++----
>> >> > ?1 files changed, 4 insertions(+), 4 deletions(-)
>> >> >
>> >> > diff --git a/common/cmd_mem.c b/common/cmd_mem.c
>> >> > index 4daa1b3..e84cc4e 100644
>> >> > --- a/common/cmd_mem.c
>> >> > +++ b/common/cmd_mem.c
>> >> > @@ -471,7 +471,7 @@ int do_mem_base (cmd_tbl_t *cmdtp, int flag, int
>> >> > argc, char * const argv[])
>> >> >
>> >> > ?int do_mem_loop (cmd_tbl_t *cmdtp, int flag, int argc, char * const
>> >> > argv[]) {
>> >> > - ? ? ? ulong ? addr, length, i, junk;
>> >> > + ? ? ? ulong ? addr, length, i;
>> >> > ? ? ? ?int ? ? size;
>> >> > ? ? ? ?volatile uint ? *longp;
>> >> > ? ? ? ?volatile ushort *shortp;
>> >> > @@ -518,7 +518,7 @@ int do_mem_loop (cmd_tbl_t *cmdtp, int flag, int
>> >> > argc, char * const argv[]) longp = (uint *)addr;
>> >> > ? ? ? ? ? ? ? ? ? ? ? ?i = length;
>> >> > ? ? ? ? ? ? ? ? ? ? ? ?while (i-- > 0)
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? junk = *longp++;
>> >> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? *longp++;
>> >> > ? ? ? ? ? ? ? ?}
>> >> > ? ? ? ?}
>> >> > ? ? ? ?if (size == 2) {
>> >> > @@ -526,14 +526,14 @@ int do_mem_loop (cmd_tbl_t *cmdtp, int flag, int
>> >> > argc, char * const argv[]) shortp = (ushort *)addr;
>> >> > ? ? ? ? ? ? ? ? ? ? ? ?i = length;
>> >> > ? ? ? ? ? ? ? ? ? ? ? ?while (i-- > 0)
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? junk = *shortp++;
>> >> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? *shortp++;
>> >> > ? ? ? ? ? ? ? ?}
>> >> > ? ? ? ?}
>> >> > ? ? ? ?for (;;) {
>> >> > ? ? ? ? ? ? ? ?cp = (u_char *)addr;
>> >> > ? ? ? ? ? ? ? ?i = length;
>> >> > ? ? ? ? ? ? ? ?while (i-- > 0)
>> >> > - ? ? ? ? ? ? ? ? ? ? ? junk = *cp++;
>> >> > + ? ? ? ? ? ? ? ? ? ? ? *cp++;
>> >> > ? ? ? ?}
>> >> > ?}
>> >>
>> >> I checked the ARM assembler output and it looks fine.
>> >>
>> >> Acked-by: Simon Glass <sjg@chromium.org>
>> >>
>> >> Re the 'length == 1' case (above your patch) site, it is assigning to
>> >> 'i' here. Could/should we remove that assignment also?
>> >
>> > The loop below depends on that ... ?
>>
>> Which loop? It doesn't look like the value of i is used?
>
> i = length;
> while (i-- > 0)
>
> This loop

The assignment to i I was referring to is here:

if (length == 1) {
if (size == 4) {
longp = (uint *)addr;
for (;;)
i = *longp;
^^^ this line

}
if (size == 2) {
shortp = (ushort *)addr;
for (;;)
i = *shortp;
^^^ this line

}
cp = (u_char *)addr;
for (;;)
i = *cp;
^^^ this line

}

I was wondering if we need to assign to i? The output code appears
unchanged with my compiler if the 'i =' is removed.

Regards,
Simon