On Mon, Jun 03, 2013 at 02:09:48PM -0500, Mark Tinguely wrote:
> On 06/03/13 00:28, Dave Chinner wrote:
> >From: Dave Chinner<dchinner@xxxxxxxxxx>
> >
> >When invalidating an attribute leaf block block, there might be
> >remote attributes that it points to. With the recent rework of the
> >remote attribute format, we have to make sure we calculate the
> >length of the attribute correctly. We aren't doing that in
> >xfs_attr3_leaf_inactive(), so fix it.
> >
> >Signed-off-by: Dave Chinner<dchinner@xxxxxxxxxx>
>
> I scratched my head reading:
>
> in xfs_attr_leaf.h:
> /*
> * Used to keep a list of "remote value" extents when unlinking an inode.
> */
> typedef struct xfs_attr_inactive_list {
> xfs_dablk_t valueblk; /* block number of value bytes */
> int valuelen; /* number of bytes in value */
> ^^^^^
> |||||
> } xfs_attr_inactive_list_t;
>
> Where "valuelen" is clearly being used as blocks.
Yeah, good point.
This is one of the reasons why I dislike comments explaining what
variables in structures mean. I didn't even look at the definition
of the structure, because it's meaning is obvious from the name of
the varaible of the code that uses it. ;)
> A more obvious name is
> the former "valueblk". Blame commit d7929ff6 for the confusion.
> Should change
> the comment and/or variable one of these days ...
Actaully, a structure that is used once and local to a single
function shouldn't be declared in a header file - if should be local
to the function. I'll fix this in a separate patch for 3.11.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
