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