[Ocfs2-devel] [TRIVIAL] [PATCH] Remove unnecessary goto statements

[Ocfs2-devel] [TRIVIAL] [PATCH] Remove unnecessary goto statements

[Goldwyn Rodrigues]

Remove unnecessary goto statements.
Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.de>

---
diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
index c30b644..c91527f 100644
--- a/fs/ocfs2/suballoc.c
+++ b/fs/ocfs2/suballoc.c
@@ -824,10 +824,8 @@ int ocfs2_reserve_cluster_bitmap_bits(struct
ocfs2_super *osb,
 					     ALLOC_NEW_GROUP);
 	if (status < 0 && status != -ENOSPC) {
 		mlog_errno(status);
-		goto bail;
 	}

-bail:
 	return status;
 }

@@ -1050,7 +1048,6 @@ static inline int
ocfs2_block_group_set_bits(handle_t *handle,
 				     group_bh);
 	if (status < 0) {
 		mlog_errno(status);
-		goto bail;
 	}

 bail:

[Ocfs2-devel] [TRIVIAL] [PATCH] Remove unnecessary goto statements

[Wengang Wang]

On 10-07-22 09:50, Goldwyn Rodrigues wrote:
> Remove unnecessary goto statements.
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.de>
> 
> ---
> diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
> index c30b644..c91527f 100644
> --- a/fs/ocfs2/suballoc.c
> +++ b/fs/ocfs2/suballoc.c
> @@ -824,10 +824,8 @@ int ocfs2_reserve_cluster_bitmap_bits(struct
> ocfs2_super *osb,
>  					     ALLOC_NEW_GROUP);
>  	if (status < 0 && status != -ENOSPC) {
>  		mlog_errno(status);
> -		goto bail;
>  	}

The duplicated check for "status != -ENOSPC" can be removed too :)
mlog_errno() is doing that check.

> 
> -bail:
>  	return status;
>  }
> 
> @@ -1050,7 +1048,6 @@ static inline int
> ocfs2_block_group_set_bits(handle_t *handle,
>  				     group_bh);
>  	if (status < 0) {
>  		mlog_errno(status);
> -		goto bail;
>  	}

Why this?
the following lines are modifying the buffer head.

1361         le16_add_cpu(&bg->bg_free_bits_count, -num_bits);
1362         while(num_bits--)
1363                 ocfs2_set_bit(bit_off++, bitmap);
1364 
1365         ocfs2_journal_dirty(handle, group_bh);
1366 
1367 bail:

For the next read of this group descriptor, we can't be sure it will be dirty
read. So that it may get wrong contents and then the following write will write
the wrong contents to disk.

regards,
wengang.
> 
>  bail:

[Ocfs2-devel] [TRIVIAL] [PATCH] Remove unnecessary goto statements

[Goldwyn Rodrigues]

On Thu, Jul 22, 2010 at 10:22 AM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
>> -bail:
>> ? ? ? return status;
>> ?}
>>
>> @@ -1050,7 +1048,6 @@ static inline int
>> ocfs2_block_group_set_bits(handle_t *handle,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?group_bh);
>> ? ? ? if (status < 0) {
>> ? ? ? ? ? ? ? mlog_errno(status);
>> - ? ? ? ? ? ? goto bail;
>> ? ? ? }
>
> Why this?
> the following lines are modifying the buffer head.
>
> 1361 ? ? ? ? le16_add_cpu(&bg->bg_free_bits_count, -num_bits);
> 1362 ? ? ? ? while(num_bits--)
> 1363 ? ? ? ? ? ? ? ? ocfs2_set_bit(bit_off++, bitmap);
> 1364
> 1365 ? ? ? ? ocfs2_journal_dirty(handle, group_bh);
> 1366
> 1367 bail:
>
> For the next read of this group descriptor, we can't be sure it will be dirty
> read. So that it may get wrong contents and then the following write will write
> the wrong contents to disk.


Not sure which codebase you are referring to. According to the patch
and the code in the Joel's git repo from kernel.org, the statement
immediately following this was a bail tag.


-- 
Goldwyn

[Ocfs2-devel] [TRIVIAL] [PATCH] Remove unnecessary goto statements

[Wengang Wang]

On 10-07-22 14:34, Goldwyn Rodrigues wrote:
> On Thu, Jul 22, 2010 at 10:22 AM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
> >> -bail:
> >> ? ? ? return status;
> >> ?}
> >>
> >> @@ -1050,7 +1048,6 @@ static inline int
> >> ocfs2_block_group_set_bits(handle_t *handle,
> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?group_bh);
> >> ? ? ? if (status < 0) {
> >> ? ? ? ? ? ? ? mlog_errno(status);
> >> - ? ? ? ? ? ? goto bail;
> >> ? ? ? }
> >
> > Why this?
> > the following lines are modifying the buffer head.
> >
> > 1361 ? ? ? ? le16_add_cpu(&bg->bg_free_bits_count, -num_bits);
> > 1362 ? ? ? ? while(num_bits--)
> > 1363 ? ? ? ? ? ? ? ? ocfs2_set_bit(bit_off++, bitmap);
> > 1364
> > 1365 ? ? ? ? ocfs2_journal_dirty(handle, group_bh);
> > 1366
> > 1367 bail:
> >
> > For the next read of this group descriptor, we can't be sure it will be dirty
> > read. So that it may get wrong contents and then the following write will write
> > the wrong contents to disk.
> 
> 
> Not sure which codebase you are referring to. According to the patch
> and the code in the Joel's git repo from kernel.org, the statement
> immediately following this was a bail tag.

Oh, sorry. I made a mistake for this.

regards,
wengang.

[Ocfs2-devel] [TRIVIAL] [PATCH] Remove unnecessary goto statements

[Sunil Mushran]

NAK. Sorry. I don't think the patch is worth it. It's not like
the compiler cannot detect this already.

On 07/22/2010 07:50 AM, Goldwyn Rodrigues wrote:
> Remove unnecessary goto statements.
> Signed-off-by: Goldwyn Rodrigues<rgoldwyn@suse.de>
>
> ---
> diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
> index c30b644..c91527f 100644
> --- a/fs/ocfs2/suballoc.c
> +++ b/fs/ocfs2/suballoc.c
> @@ -824,10 +824,8 @@ int ocfs2_reserve_cluster_bitmap_bits(struct
> ocfs2_super *osb,
>   					     ALLOC_NEW_GROUP);
>   	if (status<  0&&  status != -ENOSPC) {
>   		mlog_errno(status);
> -		goto bail;
>   	}
>
> -bail:
>   	return status;
>   }
>
> @@ -1050,7 +1048,6 @@ static inline int
> ocfs2_block_group_set_bits(handle_t *handle,
>   				     group_bh);
>   	if (status<  0) {
>   		mlog_errno(status);
> -		goto bail;
>   	}
>
>   bail:
>
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel
>