[PATCH] bcachefs: Simplify code in bch2_dev_alloc()

[PATCH] bcachefs: Simplify code in bch2_dev_alloc()

[Youling Tang]

From: Youling Tang <tangyouling@kylinos.cn>

- Remove unnecessary variable 'ret'.
- Remove unnecessary bch2_dev_free() operations.

Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
---
 fs/bcachefs/super.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/fs/bcachefs/super.c b/fs/bcachefs/super.c
index 843431e58cf5..a0e01443008b 100644
--- a/fs/bcachefs/super.c
+++ b/fs/bcachefs/super.c
@@ -1352,23 +1352,20 @@ static int bch2_dev_alloc(struct bch_fs *c, unsigned dev_idx)
 {
 	struct bch_member member = bch2_sb_member_get(c->disk_sb.sb, dev_idx);
 	struct bch_dev *ca = NULL;
-	int ret = 0;
 
-	if (bch2_fs_init_fault("dev_alloc"))
-		goto err;
+	if (bch2_fs_init_fault("dev_alloc")) {
+		bch_err(c, "dev_alloc fault injected");
+		return -EFAULT;
+	}
 
 	ca = __bch2_dev_alloc(c, &member);
 	if (!ca)
-		goto err;
+		return -BCH_ERR_ENOMEM_dev_alloc;
 
 	ca->fs = c;
 
 	bch2_dev_attach(c, ca, dev_idx);
-	return ret;
-err:
-	if (ca)
-		bch2_dev_free(ca);
-	return -BCH_ERR_ENOMEM_dev_alloc;
+	return 0;
 }
 
 static int __bch2_dev_attach_bdev(struct bch_dev *ca, struct bch_sb_handle *sb)
-- 
2.34.1

Re: [PATCH] bcachefs: Simplify code in bch2_dev_alloc()

[Kent Overstreet]

On Sat, Oct 12, 2024 at 03:59:02PM GMT, Youling Tang wrote:
> From: Youling Tang <tangyouling@kylinos.cn>
> 
> - Remove unnecessary variable 'ret'.
> - Remove unnecessary bch2_dev_free() operations.
> 
> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>

the 'goto err' style is more readable when you're staring at tons of
code

Re: [PATCH] bcachefs: Simplify code in bch2_dev_alloc()

[Youling Tang]

Hi, Kent

On 2024/10/13 01:30, Kent Overstreet wrote:

> On Sat, Oct 12, 2024 at 03:59:02PM GMT, Youling Tang wrote:
>> From: Youling Tang <tangyouling@kylinos.cn>
>>
>> - Remove unnecessary variable 'ret'.
>> - Remove unnecessary bch2_dev_free() operations.
>>
>> Signed-off-by: Youling Tang <tangyouling@kylinos.cn>
> the 'goto err' style is more readable when you're staring at tons of
> code
Well.

But 'ret' is not needed here.
Does not execute bch2_dev_free() if the ca in the err path is NULL,
so it can be removed.
Can be modified as follows if necessary:
-- a/fs/bcachefs/super.c
+++ b/fs/bcachefs/super.c
@@ -1352,7 +1352,6 @@ static int bch2_dev_alloc(struct bch_fs *c, 
unsigned dev_idx)
  {
         struct bch_member member = bch2_sb_member_get(c->disk_sb.sb, 
dev_idx);
         struct bch_dev *ca = NULL;
-       int ret = 0;

         if (bch2_fs_init_fault("dev_alloc"))
                 goto err;
@@ -1364,10 +1363,9 @@ static int bch2_dev_alloc(struct bch_fs *c, 
unsigned dev_idx)
         ca->fs = c;

         bch2_dev_attach(c, ca, dev_idx);
-       return ret;
+       return 0;
  err:
-       if (ca)
-               bch2_dev_free(ca);
         return -BCH_ERR_ENOMEM_dev_alloc;

  }

Thanks,
Youling.