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

Marek Vasut
2011-09-26T19:10:13Z

On Monday, September 26, 2011 09:01:16 PM Simon Glass wrote:
> 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.

Oh, right ... this can be removed. That code seems quite legacy and in a urgent
need of cleanup. Shall we wrap this change into this patch or do a subsequent
one ?

Cheers
>
> Regards,
> Simon