[PATCH] mpis: cavium-octeon/executive/cvmx-l2c.c: fix uninitialized variable

[PATCH] mpis: cavium-octeon/executive/cvmx-l2c.c: fix uninitialized variable

[Cong Ding]

the variable dummy is used without initialization.

Signed-off-by: Cong Ding <dinggnu@gmail.com>
---
 arch/mips/cavium-octeon/executive/cvmx-l2c.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/cavium-octeon/executive/cvmx-l2c.c b/arch/mips/cavium-octeon/executive/cvmx-l2c.c
index 9f883bf..d7e07af 100644
--- a/arch/mips/cavium-octeon/executive/cvmx-l2c.c
+++ b/arch/mips/cavium-octeon/executive/cvmx-l2c.c
@@ -286,7 +286,7 @@ uint64_t cvmx_l2c_read_perf(uint32_t counter)
 static void fault_in(uint64_t addr, int len)
 {
 	volatile char *ptr;
-	volatile char dummy;
+	volatile char dummy = 0;
 	/*
 	 * Adjust addr and length so we get all cache lines even for
 	 * small ranges spanning two cache lines.
-- 
1.7.9.5

Re: [PATCH] mpis: cavium-octeon/executive/cvmx-l2c.c: fix uninitialized variable

[Ralf Baechle]

On Mon, Jan 14, 2013 at 10:47:03PM +0100, Cong Ding wrote:

> the variable dummy is used without initialization.

Interesting - I wonder how you found this one.  My compiler (gcc 4.7)
doesn't warn about this one.

Nor does gcc notice that the whole summing up business is wasted efford.

So here's my counter proposal.  It works because ptr is a volatile pointer
so the compiler will always dereference it even if the returned value is
not being used.  The resulting code is a bit smaller.

  Ralf

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 arch/mips/cavium-octeon/executive/cvmx-l2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/cavium-octeon/executive/cvmx-l2c.c b/arch/mips/cavium-octeon/executive/cvmx-l2c.c
index 9f883bf..ec3e059 100644
--- a/arch/mips/cavium-octeon/executive/cvmx-l2c.c
+++ b/arch/mips/cavium-octeon/executive/cvmx-l2c.c
@@ -286,7 +286,7 @@ uint64_t cvmx_l2c_read_perf(uint32_t counter)
 static void fault_in(uint64_t addr, int len)
 {
 	volatile char *ptr;
-	volatile char dummy;
+
 	/*
 	 * Adjust addr and length so we get all cache lines even for
 	 * small ranges spanning two cache lines.
@@ -300,7 +300,7 @@ static void fault_in(uint64_t addr, int len)
 	 */
 	CVMX_DCACHE_INVALIDATE;
 	while (len > 0) {
-		dummy += *ptr;
+		*ptr;
 		len -= CVMX_CACHE_LINE_SIZE;
 		ptr += CVMX_CACHE_LINE_SIZE;
 	}

Re: [PATCH] mpis: cavium-octeon/executive/cvmx-l2c.c: fix uninitialized variable

[David Daney]

On 01/16/2013 02:57 AM, Ralf Baechle wrote:
> On Mon, Jan 14, 2013 at 10:47:03PM +0100, Cong Ding wrote:
>
>> the variable dummy is used without initialization.
>
> Interesting - I wonder how you found this one.  My compiler (gcc 4.7)
> doesn't warn about this one.
>

I get no warnings either.  So I wonder what the point of churning up the 
code is.

David Daney

> Nor does gcc notice that the whole summing up business is wasted efford.
>
> So here's my counter proposal.  It works because ptr is a volatile pointer
> so the compiler will always dereference it even if the returned value is
> not being used.  The resulting code is a bit smaller.
>
>    Ralf
>
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
>
>   arch/mips/cavium-octeon/executive/cvmx-l2c.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/cavium-octeon/executive/cvmx-l2c.c b/arch/mips/cavium-octeon/executive/cvmx-l2c.c
> index 9f883bf..ec3e059 100644
> --- a/arch/mips/cavium-octeon/executive/cvmx-l2c.c
> +++ b/arch/mips/cavium-octeon/executive/cvmx-l2c.c
> @@ -286,7 +286,7 @@ uint64_t cvmx_l2c_read_perf(uint32_t counter)
>   static void fault_in(uint64_t addr, int len)
>   {
>   	volatile char *ptr;
> -	volatile char dummy;
> +
>   	/*
>   	 * Adjust addr and length so we get all cache lines even for
>   	 * small ranges spanning two cache lines.
> @@ -300,7 +300,7 @@ static void fault_in(uint64_t addr, int len)
>   	 */
>   	CVMX_DCACHE_INVALIDATE;
>   	while (len > 0) {
> -		dummy += *ptr;
> +		*ptr;
>   		len -= CVMX_CACHE_LINE_SIZE;
>   		ptr += CVMX_CACHE_LINE_SIZE;
>   	}
>
>
>

Re: [PATCH] mpis: cavium-octeon/executive/cvmx-l2c.c: fix uninitialized variable

[Geert Uytterhoeven]

On Wed, Jan 16, 2013 at 11:57 AM, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Mon, Jan 14, 2013 at 10:47:03PM +0100, Cong Ding wrote:
>
>> the variable dummy is used without initialization.
>
> Interesting - I wonder how you found this one.  My compiler (gcc 4.7)
> doesn't warn about this one.

Probably older gcc does.

> Nor does gcc notice that the whole summing up business is wasted efford.
>
> So here's my counter proposal.  It works because ptr is a volatile pointer
> so the compiler will always dereference it even if the returned value is
> not being used.  The resulting code is a bit smaller.
>
>   Ralf
>
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
>
>  arch/mips/cavium-octeon/executive/cvmx-l2c.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/cavium-octeon/executive/cvmx-l2c.c b/arch/mips/cavium-octeon/executive/cvmx-l2c.c
> index 9f883bf..ec3e059 100644
> --- a/arch/mips/cavium-octeon/executive/cvmx-l2c.c
> +++ b/arch/mips/cavium-octeon/executive/cvmx-l2c.c
> @@ -286,7 +286,7 @@ uint64_t cvmx_l2c_read_perf(uint32_t counter)
>  static void fault_in(uint64_t addr, int len)
>  {
>         volatile char *ptr;
> -       volatile char dummy;
> +
>         /*
>          * Adjust addr and length so we get all cache lines even for
>          * small ranges spanning two cache lines.
> @@ -300,7 +300,7 @@ static void fault_in(uint64_t addr, int len)
>          */
>         CVMX_DCACHE_INVALIDATE;
>         while (len > 0) {
> -               dummy += *ptr;
> +               *ptr;

Alternatively, to make clearer what's intended:
  - drop the "volatile" from "ptr" and from the cast when assigning to it,
  - use "ACCESS_ONCE(*ptr)" instead of "*ptr".

>                 len -= CVMX_CACHE_LINE_SIZE;
>                 ptr += CVMX_CACHE_LINE_SIZE;
>         }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds