Source-Changes archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/sys/ufs/lfs



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




Home | Main Index | Thread Index | Old Index