[Top] [All Lists]

Re: [PATCHv2 9/10] xfs: a few more minor xfs_log_recover.c cleanups

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCHv2 9/10] xfs: a few more minor xfs_log_recover.c cleanups
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 12 Apr 2010 17:11:31 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1270852266.7840.159.camel@doink>
References: <1270852266.7840.159.camel@doink>
User-agent: Mutt/1.5.20 (2009-06-14)
On Fri, Apr 09, 2010 at 05:31:06PM -0500, Alex Elder wrote:
> Minor things that didn't seem to warrant their own individual
> patches:
> - In xlog_bread_noalign(), reorder assertions so the buffer pointer
>   is known to be non-null before attempting to dereference it.

That's OK, but if you are debugging then it'll be obvious what
happend by it being a null ptr dereference rather than an assert
failure. Hence if anything I'd just kill the ASSERT(bp)....

> - Add a more descriptive header comment for xlog_find_verify_cycle().

It just describes what the code does - I don't think it make the
code any clearer and makes it more likely that if we ever change the
code the comment will then be wrong...

> - Make a few additions to the comments in xlog_find_head().  Also
>   rearrange some expressions in a few spots to produce the same
>   result, but in a way that seems more clear what's being computed.

I'd say this is probably the only bits of the patch that add value.
Can you split this one out by itself?


Dave Chinner

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