xfs
[Top] [All Lists]

Re: potential argument order bug in fs/xfs/xfs_dir2_node.c:xfs_dir2_leaf

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: potential argument order bug in fs/xfs/xfs_dir2_node.c:xfs_dir2_leafn_unbalance
From: Dave Jones <davej@xxxxxxxxxx>
Date: Wed, 4 Sep 2013 23:24:54 -0400
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130905031128.GZ23571@dastard>
References: <20130905023818.GA10694@xxxxxxxxxx> <20130905031128.GZ23571@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Sep 05, 2013 at 01:11:28PM +1000, Dave Chinner wrote:
 > On Wed, Sep 04, 2013 at 10:38:18PM -0400, Dave Jones wrote:
 > > I'm picking through some of the bugs in coverity's database,
 > > and I came across this one, which I'm unsure of..
 > > 
 > > In xfs_dir2_leafn_unbalance we have this code..
 > > 
 > > 1583         if (xfs_dir2_leafn_order(save_blk->bp, drop_blk->bp))
 > > 1584                 xfs_dir3_leafn_moveents(args, drop_blk->bp, &drophdr, 
 > > dents, 0,
 > > 1585                                         save_blk->bp, &savehdr, 
 > > sents, 0,
 > > 1586                                         drophdr.count);
 > > 1587         else
 > > 1588                 xfs_dir3_leafn_moveents(args, drop_blk->bp, &drophdr, 
 > > dents, 0,
 > > 1589                                         save_blk->bp, &savehdr, sents,
 > > 1590                                         savehdr.count, drophdr.count);
 > > 
 > > The issue that coverity picked up in both cases, is that 'sents' and 
 > > 'dents' are in
 > > a different order to how the xfs_dir3_leafn_moveents function expects them.
 > 
 > What does "order" mean to coverity? Is it really complaining about
 > function parameters being ordered (src, dst) rather than (dst, src)?
 > Or is it detecting that we are passing parameters names (dxxx, sxxx)
 > into a function that declares those parameters (syyy, dyyy) and it
 > throws based on that?

Yeah, the latter. It's done it to quite a few parts of the kernel.
In most cases I've looked through so far, it's not a problem, but there have
been 1-2 real bugs.

 > In more detail, the function prototype is effectively
 > xfs_dir3_leafn_moveents(source, destination, count), and so in both
 > cases here objects are being moved from the block being dropped
 > (freed) to the block being saved (merged block).

Ok, thanks for looking it over anyway.
I've marked it as being intentional in their db, so it shouldn't show up in 
future.

        Dave

<Prev in Thread] Current Thread [Next in Thread>