Subject: Re: CVS commit: src/sys/ufs/lfs
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Tyler Retzlaff <rtr@omicron-persei-8.net>
List: source-changes
Date: 03/19/2006 20:00:09
YAMAMOTO Takashi wrote:

>>>>Module Name:	src
>>>>Committed By:	rtr
>>>>Date:		Sun Mar 19 04:10:03 UTC 2006
>>>>
>>>>Modified Files:
>>>>	src/sys/ufs/lfs: lfs_syscalls.c
>>>>
>>>>Log Message:
>>>>init struct vnode *vp = NULL
>>>>coverity 2724 / run 6
>>>>XXX in future runs coverity may complain about deref NULL now but comment
>>>>   on line 382 indicates this should not be possible
>>>>   
>>>>
>>>>        
>>>>
>>>is it a real fix, or just to appease coverity?
>>> 
>>>
>>>      
>>>
>>It is a real fix.  The XXX is just an indicator that coverity will 
>>probably complain about
>>NULL deref now which could be avoided but then that would only be for 
>>the purpose of appeasing coverity.
>>    
>>
>  
>
If you want to reverse the change then please do so.  However before you 
do consider the scenario where a maintainer of this code in the future 
makes a change to the blocks of code before line 382 which results in a 
path that does not initialize vp.  It could very well end up going 
unnoticed and the uninitialized pointer being used.  If that were to 
happen it could result in a much more difficult to find bug than if vp 
had been set to NULL explicitly.

Yes, it has appeased a coverity complaint.  But it is a more defensive 
approach.  It  has not changed the semantics of the code.  Nor will it 
have (with significance) made the code slower or the resulting binary 
larger.

The only thing I have reconsidered about the change is the fact that it 
is initialized when it is declared which is against the advice of 
misc/style and whether or not the other pointers should have been 
initialized as well.

I can see perceived advantage to the change but no disadvantages.  If 
you could highlight what potential disadvantages there are then I would 
be happy to reconsider.

>YAMAMOTO Takashi
>  
>

Tyler