From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Mon, 26 Sep 2011 21:10:13 +0200 Subject: [U-Boot] [PATCH 7/7] GCC4.6: Squash warning in cmd_mem.c In-Reply-To: References: <1316996766-14248-1-git-send-email-marek.vasut@gmail.com> <201109262034.48662.marek.vasut@gmail.com> Message-ID: <201109262110.14195.marek.vasut@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 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 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 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 > >> >> > --- > >> >> > 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 > >> >> > >> >> 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