Skip to contentSkip to conversation

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

PATCH In review Cong Ding 4 participants 4 messages Jan 14, 2013 – Jan 16, 2013 MIPS

lore
Share & tools
lore

Cong Ding

author
#0 lore raw

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

the variable dummy is used without initialization.

Signed-off-by Cong Ding <[email protected]>
---
 arch/mips/cavium-octeon/executive/cvmx-l2c.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/mips/cavium-octeon/executive/cvmx-l2c.c +1 -1
index 9f883bf..d7e07af 100644
@@ -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.
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 1 line · Cong Ding

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 <[email protected]>
 arch/mips/cavium-octeon/executive/cvmx-l2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/mips/cavium-octeon/executive/cvmx-l2c.c +2 -2
index 9f883bf..ec3e059 100644
@@ -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;
 	}
#2 lore raw
Quoted 7 lines · Ralf Baechle
Quoted 1 line · Cong Ding

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

Quoted 38 lines

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 <[email protected]>
  arch/mips/cavium-octeon/executive/cvmx-l2c.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
arch/mips/cavium-octeon/executive/cvmx-l2c.c +2 -2
index 9f883bf..ec3e059 100644
@@ -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;
  	}
#3 lore raw
In reply to Ralf Baechle
Quoted 6 lines · Ralf Baechle
Quoted 1 line · Cong Ding

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.

Quoted 27 lines

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 <[email protected]>
 arch/mips/cavium-octeon/executive/cvmx-l2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/mips/cavium-octeon/executive/cvmx-l2c.c +1 -1
index 9f883bf..ec3e059 100644
@@ -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.

arch/mips/cavium-octeon/executive/cvmx-l2c.c static void fault_in(uint64_t addr, int len) · line 303

Quoted 5 lines
arch/mips/cavium-octeon/executive/cvmx-l2c.c +1 -1
@@ -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".

arch/mips/cavium-octeon/executive/cvmx-l2c.c static void fault_in(uint64_t addr, int len) · line 306

Quoted 3 lines
arch/mips/cavium-octeon/executive/cvmx-l2c.c +0 -0
                len -= CVMX_CACHE_LINE_SIZE;
                ptr += CVMX_CACHE_LINE_SIZE;
        }

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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