Re: [PATCH 4/4] futex: warning corrections

[Darren Hart <dvhart@linux.intel.com>]

On 07/07/2011 05:39 AM, Vitaliy Ivanov wrote:
>>> From 8eeaa5a97697bcc606aea23d32028aea7b271a96 Mon Sep 17 00:00:00 2001
>>> From: Vitaliy Ivanov <vitalivanov@gmail.com>
>>> Date: Thu, 7 Jul 2011 00:05:05 +0300
>>> Subject: [PATCH] futex: uninitialized warning corrections
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> kernel/futex.c: In function ‘fixup_pi_state_owner.clone.17’:
>>> kernel/futex.c:1582:6: warning: ‘curval’ may be used uninitialized in this function
>>> kernel/futex.c: In function ‘handle_futex_death’:
>>> kernel/futex.c:2486:6: warning: ‘nval’ may be used uninitialized in this function
>>> kernel/futex.c: In function ‘do_futex’:
>>> kernel/futex.c:863:11: warning: ‘curval’ may be used uninitialized in this function
>>> kernel/futex.c:828:6: note: ‘curval’ was declared here
>>> kernel/futex.c:898:5: warning: ‘oldval’ may be used uninitialized in this function
>>> kernel/futex.c:890:6: note: ‘oldval’ was declared here
>>>
>>> Signed-off-by: Vitaliy Ivanov <vitalivanov@gmail.com>
>>
>> Please include a blurb in the commit message as to why you used
>> uninitialized_var() rather than just assigning it. This will save people
>> the time of wondering why, and me the time of nacking "it's simpler to
>> just initialize to zero" patches :-)
>>
>> Acked-by: Darren Hart <dvhart@linux.intel.com>
> 
> Darren,
> 
> Thanks for your comments. I think the description is pretty obvious
> here as I don't think any of these variables are affected by cmpxchg.

Not so. Consider the following:

	u32 curval;
	if (cmpxchg_futex_value_locked(&curval, uaddr, uval, newval))
		ret = -EFAULT;
	else if (curval != uval)
		ret = -EINVAL;

the cmpxchg here assigns curval to newval if *uaddr==uval or to *uaddr
otherwise. This is where curval gets assigned so that it can then be
read in the following if block. gcc didn't recognize this as an
assignment and is why it complained about it being used uninitialized.


> There is simple assignment at the end. Seems like compiler simply
> doesn't follow all the return cases.

No, the compiler complained about the test of the value, this doesn't
have anything to do with the return cases.

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel