diff mbox series

[v2,1/3] btrfs: remove pointless out label in find_first_block_group

Message ID 20200527081227.3408-2-johannes.thumshirn@wdc.com (mailing list archive)
State New, archived
Headers show
Series 3 small cleanups for find_first_block_group | expand

Commit Message

Johannes Thumshirn May 27, 2020, 8:12 a.m. UTC 
The 'out' label in find_first_block_group() does not do anything in terms
of cleanup.

It is better to directly return 'ret' instead of jumping to out to not
confuse readers. Additionally there is no need to initialize ret with 0.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/block-group.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

David Sterba May 27, 2020, 1:06 p.m. UTC | #1 
On Wed, May 27, 2020 at 05:12:25PM +0900, Johannes Thumshirn wrote:
> The 'out' label in find_first_block_group() does not do anything in terms
> of cleanup.
> 
> It is better to directly return 'ret' instead of jumping to out to not
> confuse readers. Additionally there is no need to initialize ret with 0.

https://www.kernel.org/doc/html/latest/process/coding-style.html#centralized-exiting-of-functions
Johannes Thumshirn May 27, 2020, 1:45 p.m. UTC | #2 
On 27/05/2020 15:07, David Sterba wrote:
> On Wed, May 27, 2020 at 05:12:25PM +0900, Johannes Thumshirn wrote:
>> The 'out' label in find_first_block_group() does not do anything in terms
>> of cleanup.
>>
>> It is better to directly return 'ret' instead of jumping to out to not
>> confuse readers. Additionally there is no need to initialize ret with 0.
> 
> https://www.kernel.org/doc/html/latest/process/coding-style.html#centralized-exiting-of-functions
> 

To cite the Link you posted:

"The goto statement comes in handy when a function exits from multiple 
locations and some common work such as cleanup has to be done. If there 
is no cleanup needed then just return directly."
David Sterba June 1, 2020, 3:34 p.m. UTC | #3 
On Wed, May 27, 2020 at 01:45:47PM +0000, Johannes Thumshirn wrote:
> On 27/05/2020 15:07, David Sterba wrote:
> > On Wed, May 27, 2020 at 05:12:25PM +0900, Johannes Thumshirn wrote:
> >> The 'out' label in find_first_block_group() does not do anything in terms
> >> of cleanup.
> >>
> >> It is better to directly return 'ret' instead of jumping to out to not
> >> confuse readers. Additionally there is no need to initialize ret with 0.
> > 
> > https://www.kernel.org/doc/html/latest/process/coding-style.html#centralized-exiting-of-functions
> > 
> 
> To cite the Link you posted:
> 
> "The goto statement comes in handy when a function exits from multiple 
> locations and some common work such as cleanup has to be done. If there 
> is no cleanup needed then just return directly."

The preference in btrfs code has been to avoid returns from the middle
of while loops. The kernel coding style does not cover everything, we've
settled on some style in btrfs and that's what I'm arguing for and
fixing up in many patches so it's consistent.
diff mbox series

Patch

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 176e8a292fd1..c5acd89c864c 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -1527,7 +1527,7 @@  static int find_first_block_group(struct btrfs_fs_info *fs_info,
 				  struct btrfs_key *key)
 {
 	struct btrfs_root *root = fs_info->extent_root;
-	int ret = 0;
+	int ret;
 	struct btrfs_key found_key;
 	struct extent_buffer *leaf;
 	struct btrfs_block_group_item bg;
@@ -1536,7 +1536,7 @@  static int find_first_block_group(struct btrfs_fs_info *fs_info,
 
 	ret = btrfs_search_slot(NULL, root, key, path, 0, 0);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	while (1) {
 		slot = path->slots[0];
@@ -1546,7 +1546,7 @@  static int find_first_block_group(struct btrfs_fs_info *fs_info,
 			if (ret == 0)
 				continue;
 			if (ret < 0)
-				goto out;
+				return ret;
 			break;
 		}
 		btrfs_item_key_to_cpu(leaf, &found_key, slot);
@@ -1594,11 +1594,11 @@  static int find_first_block_group(struct btrfs_fs_info *fs_info,
 				}
 			}
 			free_extent_map(em);
-			goto out;
+			return ret;
 		}
 		path->slots[0]++;
 	}
-out:
+
 	return ret;
 }