Re: [KJ] [PATCH] Fix gcc4 warning, def & len may be used uninitialized

Re: [KJ] [PATCH] Fix gcc4 warning, def & len may be used uninitialized

[Mark Hollomon]

Jesse Millan wrote:
> 
> --- 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
> @@ -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;
> +	}
> 
> -	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?

-- 
Mark Hollomon
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Fix gcc4 warning, def & len may be used uninitialized

[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.

Mark Hollomon wrote:
> Jesse Millan wrote:
> 
>>
>> --- 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
>> @@ -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;
>> +    }
>>
>> -    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?
> 

-- 
Jesse Millan
CNS Unix Team
Portland State University
Phone: (503) 725-9151
Mobile: (503) 453-0748
GPG key: www.system-calls.com/gpg.php

grep --recursive --ignore-case 'SHOULD WORK' /usr/src/linux/* | wc
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [PATCH] Fix gcc4 warning, def & len may be used uninitialized

[Jesse Millan]

From now on, Ill initialize the parameters at the top/start of the
callee, unless it does not make sense to do so.

Thanks for the input.

Arnd Bergmann wrote:
> On Middeweken 25 Mai 2005 21:18, Jesse Millan wrote:
> 
>>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 <><
> 

-- 
Jesse Millan
CNS Unix Team
Portland State University
Phone: (503) 725-9151
Mobile: (503) 453-0748
GPG key: www.system-calls.com/gpg.php

grep --recursive --ignore-case 'SHOULD WORK' /usr/src/linux/* | wc
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors