xfs
[Top] [All Lists]

Re: [PATCHv2 7/10] xfs: tidy up some goto labels

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCHv2 7/10] xfs: tidy up some goto labels
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Mon, 12 Apr 2010 16:52:32 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1270852163.7840.154.camel@doink>
References: <1270852163.7840.154.camel@doink>
User-agent: Mutt/1.5.20 (2009-06-14)
On Fri, Apr 09, 2010 at 05:29:23PM -0500, Alex Elder wrote:
> Rename a label used in xlog_find_head() that I thought was poorly
> chosen.  Also combine two adjacent labels xlog_find_tail() into
> a single label, and give it a more generic name.
> 
> Signed-off-by: Alex Elder <aelder@xxxxxxx>
> 
> ---
> fs/xfs/xfs_log_recover.c |   83 
> ++++++++++++++++++++++++-----------------------
>  fs/xfs/xfs_log_recover.c |   20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> Index: b/fs/xfs/xfs_log_recover.c
> ===================================================================
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -739,7 +739,7 @@ xlog_find_head(
>                       goto bp_err;
>               if (new_blk != -1) {
>                       head_blk = new_blk;
> -                     goto bad_blk;
> +                     goto fine_tune;
>               }
>  
>               /*
> @@ -757,7 +757,7 @@ xlog_find_head(
>                       head_blk = new_blk;
>       }
>  
> - bad_blk:
> +fine_tune:
>       /*
>        * Now we need to make sure head_blk is not pointing to a block in
>        * the middle of a log record.

I don't think "fine_tune" really matches what is being done here
either. "bad_blk" makes sense when you consider that the search is
being terminated due to a log block being found that didn't match
the search criteria. i.e. it is bad.

What we are really doing there at "bad_blk" is validating the head
block we have found, so if you are going to change the label then
"validate_head" makes more sense to me...

> @@ -864,12 +864,12 @@ xlog_find_tail(
>       if (*head_blk == 0) {                           /* special case */
>               error = xlog_bread(log, 0, 1, bp, &offset);
>               if (error)
> -                     goto bread_err;
> +                     goto done;
>  
>               if (xlog_get_cycle(offset) == 0) {
>                       *tail_blk = 0;
>                       /* leave all other log inited values alone */
> -                     goto exit;
> +                     goto done;
>               }
>       }

These changes look fine, though.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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