Fix gcc4 warning, def & len may be used uninitialized

fs/cifs/asn1.c renamed from fs/cifs/asn1.c~ +5 -1

@@ -160,12 +160,18 @@ asn1_length_decode(struct asn1_ctx *ctx,
 {
 	unsigned char ch, cnt;
 
-	if (!asn1_octet_decode(ctx, &ch))
+	if (!asn1_octet_decode(ctx, &ch)) {
+		/* Function would have returned without initializing 'def' and 'len' */
+		*def = 0;
+		*len = 0;
 		return 0;
+	}
 

Quoted 20 lines · Jesse Millan

--- linux-2.6.12-rc4/fs/cifs/asn1.c~ 2005-05-24 22:25:21.436866468
-0700
+++ linux-2.6.12-rc4/fs/cifs/asn1.c 2005-05-24 22:49:43.744939729
-0700

(quoted diff) +2 -1

@@ -160,12 +160,18 @@ asn1_length_decode(struct asn1_ctx *ctx,
 {
     unsigned char ch, cnt;
 
-    if (!asn1_octet_decode(ctx, &ch))
+    if (!asn1_octet_decode(ctx, &ch)) {
+        /* Function would have returned without initializing 'def'

and 'len' */

(quoted diff) +3 -0

+        *def = 0;
+        *len = 0;
         return 0;
+    }
 

Review

Quoted 10 lines

(quoted diff) +4 -2

-    if (ch = 0x80)
+    if (ch = 0x80) {
         *def = 0;
-    else {
+        /* Function would have returned without initializing 'len' */
+        *len = 0;
+    } else {
         *def = 1;
 
         if (ch < 0x80)

Wouldn't it be better to just initialize def and len at the top of the function and be done? Or does that violate some kernel style preference?

Quoted 3 lines · Jesse Millan

Your right, it could go either way. Arnd Bergmann had posted earlier and said that it would be better to make the change in the function itself. I took that advice.

I think what Mark was suggesting is to initialize the parameters at the start of the inner function (asn1_length_decode), which makes perfect sense to me, at least in this particular case.

What you should not do is initialize it in the calling function, because then you defeat gcc's ability to warn you about real uninitialized uses of the local variables in that function.

	Arnd <><