| Message ID | 1316996766-14248-7-git-send-email-marek.vasut@gmail.com |
|---|---|
| State | Accepted |
| Commit | f3b3c3df189f4d35c903c3472619017d0bd1baa7 |
| Headers | show |
On Sunday, September 25, 2011 20:26:06 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. i think the stores to a var were added to avoid a gcc warning, but if newer versions don't warn anymore, should be fine. Acked-by: Mike Frysinger <vapier@gentoo.org> -mike
Dear Mike Frysinger, In message <201109252341.30328.vapier@gentoo.org> you wrote: > > > The assigned variable can be removed because the pointers are volatile so > > accesses to their addresses are always generated. > > i think the stores to a var were added to avoid a gcc warning, but if newer > versions don't warn anymore, should be fine. Obviously we have to check if older tools still accept this. Marek, which tool chains did you use for testing? Best regards, Wolfgang Denk
On Monday, September 26, 2011 09:25:36 AM Wolfgang Denk wrote: > Dear Mike Frysinger, > > In message <201109252341.30328.vapier@gentoo.org> you wrote: > > > The assigned variable can be removed because the pointers are volatile > > > so accesses to their addresses are always generated. > > > > i think the stores to a var were added to avoid a gcc warning, but if > > newer versions don't warn anymore, should be fine. > > Obviously we have to check if older tools still accept this. > Hi, as I was never completely sure about this file from the begining, I'm all for it. Btw. who's supposed to merge the rest of the series anyway ? > > Marek, which tool chains did you use for testing? Hand-built gcc4.6.0+debian patches and gcc4.4.6+debian patches: arm-linux-gnueabi-gcc-4.4 (Debian 4.4.6-7) 4.4.6 arm-linux-gnueabi-gcc-4.6 (Debian 4.6.0-14) 4.6.1 20110616 (prerelease) There was someone on the irc who (unsuccessfully) tried to compile uboot with gcc 2.xx recently. Maybe we should impose some lower bound for compiler version, though I guess 4.4 is too high. Suggestions ? Cheers > > Best regards, > > Wolfgang Denk
On Monday, September 26, 2011 05:03:46 Marek Vasut wrote: > On Monday, September 26, 2011 09:25:36 AM Wolfgang Denk wrote: > > Mike Frysinger wrote: > > > > The assigned variable can be removed because the pointers are > > > > volatile so accesses to their addresses are always generated. > > > > > > i think the stores to a var were added to avoid a gcc warning, but if > > > newer versions don't warn anymore, should be fine. > > > > Obviously we have to check if older tools still accept this. > > Hi, > > as I was never completely sure about this file from the begining, I'm all > for it. > > Btw. who's supposed to merge the rest of the series anyway ? > > > Marek, which tool chains did you use for testing? > > Hand-built gcc4.6.0+debian patches and gcc4.4.6+debian patches: > > arm-linux-gnueabi-gcc-4.4 (Debian 4.4.6-7) 4.4.6 > arm-linux-gnueabi-gcc-4.6 (Debian 4.6.0-14) 4.6.1 20110616 (prerelease) > > There was someone on the irc who (unsuccessfully) tried to compile uboot > with gcc 2.xx recently. Maybe we should impose some lower bound for > compiler version, though I guess 4.4 is too high. Suggestions ? i think gcc-3.x has been broken for a while but no one has noticed -mike
On Monday, September 26, 2011 06:10:37 PM Mike Frysinger wrote: > On Monday, September 26, 2011 05:03:46 Marek Vasut wrote: > > On Monday, September 26, 2011 09:25:36 AM Wolfgang Denk wrote: > > > Mike Frysinger wrote: > > > > > The assigned variable can be removed because the pointers are > > > > > volatile so accesses to their addresses are always generated. > > > > > > > > i think the stores to a var were added to avoid a gcc warning, but if > > > > newer versions don't warn anymore, should be fine. > > > > > > Obviously we have to check if older tools still accept this. > > > > Hi, > > > > as I was never completely sure about this file from the begining, I'm all > > for it. > > > > Btw. who's supposed to merge the rest of the series anyway ? > > > > > Marek, which tool chains did you use for testing? > > > > Hand-built gcc4.6.0+debian patches and gcc4.4.6+debian patches: > > > > arm-linux-gnueabi-gcc-4.4 (Debian 4.4.6-7) 4.4.6 > > arm-linux-gnueabi-gcc-4.6 (Debian 4.6.0-14) 4.6.1 20110616 (prerelease) > > > > There was someone on the irc who (unsuccessfully) tried to compile uboot > > with gcc 2.xx recently. Maybe we should impose some lower bound for > > compiler version, though I guess 4.4 is too high. Suggestions ? > > i think gcc-3.x has been broken for a while but no one has noticed I guess we should go the kernel way -- make it 4.2 and be done with it. > -mike
Dear Marek Vasut, In message <201109261931.24045.marek.vasut@gmail.com> you wrote: > > > i think gcc-3.x has been broken for a while but no one has noticed > > I guess we should go the kernel way -- make it 4.2 and be done with it. You are off by one major version. The kernel README says: Make sure you have at least gcc 3.2 available. Note that this will NOT build any half-way recent U-Boot version. Best regards, Wolfgang Denk
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? Regards, Simon > > -- > 1.7.5.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
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 ... ? > > Regards, > Simon > > > -- > > 1.7.5.4 > > > > _______________________________________________ > > U-Boot mailing list > > U-Boot@lists.denx.de > > http://lists.denx.de/mailman/listinfo/u-boot
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? Regards, Simon > >> >> Regards, >> Simon >> >> > -- >> > 1.7.5.4 >> > >> > _______________________________________________ >> > U-Boot mailing list >> > U-Boot@lists.denx.de >> > http://lists.denx.de/mailman/listinfo/u-boot >
On Monday, September 26, 2011 08:03:45 PM Wolfgang Denk wrote: > Dear Marek Vasut, > > In message <201109261931.24045.marek.vasut@gmail.com> you wrote: > > > i think gcc-3.x has been broken for a while but no one has noticed > > > > I guess we should go the kernel way -- make it 4.2 and be done with it. > > You are off by one major version. The kernel README says: > > Make sure you have at least gcc 3.2 available. > > Note that this will NOT build any half-way recent U-Boot version. It might have been my imagination then ... but 4.2 sounded quite logical to me, sorry for confusion. Anyway, is there still anyone using 3.x ? If there isn't any such users, we should simply discard it, add gcc version check and set the lower bound to some not-too-buggy version of gcc4. Best regards, Marek Vasut
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 > > Regards, > Simon > > >> Regards, > >> Simon > >> > >> > -- > >> > 1.7.5.4 > >> > > >> > _______________________________________________ > >> > U-Boot mailing list > >> > U-Boot@lists.denx.de > >> > http://lists.denx.de/mailman/listinfo/u-boot
Dear Marek Vasut, In message <201109262029.24934.marek.vasut@gmail.com> you wrote: > > Anyway, is there still anyone using 3.x ? If there isn't any such users, we > should simply discard it, add gcc version check and set the lower bound to some > not-too-buggy version of gcc4. To be honest: yes, there are users. We recently had customers where we had to dig out ELDK 2.1.0 (gcc 2.95) to build thir code. But obviously these don't use any recent code either. Best regards, Wolfgang Denk
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
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
Hi Marek, On Mon, Sep 26, 2011 at 12:10 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > On Monday, September 26, 2011 09:01:16 PM Simon Glass wrote: >> Hi Marek, >> >> >> 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 ? I'm sure you know better than me, but it feels like a separate commit if only because your commit msg is about GCC 4.6 warnings. Regards, Simon > > Cheers
On Monday, September 26, 2011 14:29:24 Marek Vasut wrote: > On Monday, September 26, 2011 08:03:45 PM Wolfgang Denk wrote: > > Marek Vasut wrote: > > > > i think gcc-3.x has been broken for a while but no one has noticed > > > > > > I guess we should go the kernel way -- make it 4.2 and be done with it. > > > > You are off by one major version. The kernel README says: > > Make sure you have at least gcc 3.2 available. > > > > Note that this will NOT build any half-way recent U-Boot version. > > It might have been my imagination then ... but 4.2 sounded quite logical to > me, sorry for confusion. > > Anyway, is there still anyone using 3.x ? If there isn't any such users, we > should simply discard it, add gcc version check and set the lower bound to > some not-too-buggy version of gcc4. we had someone post a fix for using gcc-3.x in current master -mike
Dear Marek Vasut, In message <1316996766-14248-7-git-send-email-marek.vasut@gmail.com> you 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(-) Applied, thanks. Best regards, Wolfgang Denk
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++; } }
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(-)