Hi Jesper,
On Wed, Aug 16, 2006 at 11:27:34PM +0200, Jesper Juhl wrote:
> (Please keep me on Cc since I'm not subscribed to the XFS lists)
>
> The coverity checker found what looks to me like a valid case of
> potentially uninitialized variable use (see below).
It looks invalid, but its not, once again. To understand why this
isn't a problem requires looking at the xfs_alloc_ag_vextent_small
call sites (there's only two). If (*flen==0) is passed back out,
then the value in *fbno is discarded, always.
> So basically, if we hit the 'else' branch, then 'fbno' has not been
> initialized and line 1490 will then use that uninitialized variable.
>
> What would prevent that from happening at some time??
Nothing. But its not a problem in practice. However, that final
else branch is very much unlikely, so theres no real cost to just
initialising the local fbno to NULLAGBLOCK in that branch, and we
future proof ourselves a bit that way I guess (in case the callers
ever change - pretty unlikely, but we may as well). How does the
patch below look to you?
cheers.
--
Nathan
Index: xfs-linux/xfs_alloc.c
===================================================================
--- xfs-linux.orig/xfs_alloc.c 2006-08-17 08:27:26.807943000 +1000
+++ xfs-linux/xfs_alloc.c 2006-08-17 08:39:55.126710000 +1000
@@ -1478,8 +1478,10 @@ xfs_alloc_ag_vextent_small(
/*
* Can't allocate from the freelist for some reason.
*/
- else
+ else {
+ fbno = NULLAGBLOCK;
flen = 0;
+ }
/*
* Can't do the allocation, give up.
*/
|