btrfs: remove unnecessary error check
PATCH 1/1 btrfs: remove unnecessary error check
From: Satoru Takeuchi <[email protected]>
If "(!IS_ERR(trans) || PTR_ERR(trans) != -ENOSPC))" is false, obviously "trans" is -ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans) == -ENOSPC)" check.
--- fs/btrfs/inode.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)
fs/btrfs/inode.c +13 -14
@@ -3803,22 +3803,21 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir)if (!IS_ERR (trans) ||PTR_ERR (trans) != -ENOSPC )return trans; -if (PTR_ERR (trans) == -ENOSPC ) { -u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); +u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); - trans =btrfs_start_transaction (root,0 ); -if (IS_ERR (trans)) -return trans; - ret =btrfs_cond_migrate_bytes (root->fs_info, - &root->fs_info->trans_block_rsv, - num_bytes,5 ); -if (ret) { -btrfs_end_transaction (trans, root); -return ERR_PTR (ret); - } - trans->block_rsv = &root->fs_info->trans_block_rsv; - trans->bytes_reserved = num_bytes; + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return trans; + ret =btrfs_cond_migrate_bytes (root->fs_info, + &root->fs_info->trans_block_rsv, + num_bytes,5 ); +if (ret) { +btrfs_end_transaction (trans, root); +return ERR_PTR (ret); } + trans->block_rsv = &root->fs_info->trans_block_rsv; + trans->bytes_reserved = num_bytes; +return trans; }
Signature · 1 line
1.9.3
Quoted 10 lines · Satoru Takeuchi
From: Satoru Takeuchi <[email protected]>
If "(!IS_ERR(trans) ||
PTR_ERR(trans)!=-ENOSPC))" is false, obviously "trans" is-ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans)==-ENOSPC)" check.
True, but now a comment like:
/* Handle ENOSPC */
might still be nice...
Eric
Quoted 53 lines
Signed-off-by Satoru Takeuchi <[email protected]>---
fs/btrfs/inode.c| 27 +++++++++++++--------------1 file changed, 13 insertions(+), 14 deletions(-)fs/btrfs/inode.c +13
-14@@ -3803,22 +3803,21 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir)if (!IS_ERR (trans) ||PTR_ERR (trans) != -ENOSPC )return trans; -if (PTR_ERR (trans) == -ENOSPC ) { -u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); +u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); - trans =btrfs_start_transaction (root,0 ); -if (IS_ERR (trans)) -return trans; - ret =btrfs_cond_migrate_bytes (root->fs_info, - &root->fs_info->trans_block_rsv, - num_bytes,5 ); -if (ret) { -btrfs_end_transaction (trans, root); -return ERR_PTR (ret); - } - trans->block_rsv = &root->fs_info->trans_block_rsv; - trans->bytes_reserved = num_bytes; + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return trans; + ret =btrfs_cond_migrate_bytes (root->fs_info, + &root->fs_info->trans_block_rsv, + num_bytes,5 ); +if (ret) { +btrfs_end_transaction (trans, root); +return ERR_PTR (ret); } + trans->block_rsv = &root->fs_info->trans_block_rsv; + trans->bytes_reserved = num_bytes; +return trans; }Signature · 6 lines
1.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH v2 1/1 btrfs: remove unnecessary error check
Quoted 11 lines · Eric Sandeen
Quoted 10 lines · Satoru Takeuchi
From: Satoru Takeuchi <[email protected]>
If "(!IS_ERR(trans) ||
PTR_ERR(trans)!=-ENOSPC))" is false, obviously "trans" is-ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans)==-ENOSPC)" check.
Quoted 5 lines
True, but now a comment like:
/* Handle ENOSPC */
might still be nice...
Eric, thank you for your comment. I fixed my patch. How about is it?
===
From: Satoru Takeuchi <[email protected]>
If "(!IS_ERR(trans) || PTR_ERR(trans) != -ENOSPC))" is false, obviously "trans" is -ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans) == -ENOSPC)" check.
--- fs/btrfs/inode.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)
fs/btrfs/inode.c +15 -14
@@ -3803,22 +3803,23 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir)if (!IS_ERR (trans) ||PTR_ERR (trans) != -ENOSPC )return trans; -if (PTR_ERR (trans) == -ENOSPC ) { -u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); +/* Handle ENOSPC */ - trans =btrfs_start_transaction (root,0 ); -if (IS_ERR (trans)) -return trans; - ret =btrfs_cond_migrate_bytes (root->fs_info, - &root->fs_info->trans_block_rsv, - num_bytes,5 ); -if (ret) { -btrfs_end_transaction (trans, root); -return ERR_PTR (ret); - } - trans->block_rsv = &root->fs_info->trans_block_rsv; - trans->bytes_reserved = num_bytes; +u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); + + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return trans; + ret =btrfs_cond_migrate_bytes (root->fs_info, + &root->fs_info->trans_block_rsv, + num_bytes,5 ); +if (ret) { +btrfs_end_transaction (trans, root); +return ERR_PTR (ret); } + trans->block_rsv = &root->fs_info->trans_block_rsv; + trans->bytes_reserved = num_bytes; +return trans; }
Signature · 1 line
1.9.3
Quoted 12 lines · Satoru Takeuchi
Quoted 11 lines · Eric Sandeen
Quoted 10 lines · Satoru Takeuchi
From: Satoru Takeuchi <[email protected]>
If "(!IS_ERR(trans) ||
PTR_ERR(trans)!=-ENOSPC))" is false, obviously "trans" is-ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans)==-ENOSPC)" check.
Quoted 8 lines
Quoted 5 lines
True, but now a comment like:
/* Handle ENOSPC */
might still be nice...
Eric, thank you for your comment. I fixed my patch. How about is it?
One other thing I missed the first time, I'm sorry, notes below:
Quoted 29 lines
===
From: Satoru Takeuchi <[email protected]>If "(!IS_ERR(trans) ||
PTR_ERR(trans)!=-ENOSPC))" is false, obviously "trans" is-ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans)==-ENOSPC)" check.Signed-off-by Satoru Takeuchi <[email protected]>--- fs/btrfs/inode.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)fs/btrfs/inode.c +1
-5@@ -3803,22 +3803,23 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir)if (!IS_ERR (trans) ||PTR_ERR (trans) != -ENOSPC )return trans; -if (PTR_ERR (trans) == -ENOSPC ) { -u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); +/* Handle ENOSPC */ - trans =btrfs_start_transaction (root,0 ); -if (IS_ERR (trans)) -return trans;
fs/btrfs/inode.c static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir) · line 3808
Quoted 10 lines
fs/btrfs/inode.c +1
-9- ret =btrfs_cond_migrate_bytes (root->fs_info, - &root->fs_info->trans_block_rsv, - num_bytes,5 ); -if (ret) { -btrfs_end_transaction (trans, root); -return ERR_PTR (ret); - } - trans->block_rsv = &root->fs_info->trans_block_rsv; - trans->bytes_reserved = num_bytes; +u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 );
This variable should be declared at the beginning of the function, not in the middle, because it's no longer in a separate code block.
Also, somehow by the time the patch got here, tabs turned into 4 spaces, so this one wouldn't apply for me.
Sorry for missing the variable declaration problem the first time!
-Eric
Quoted 17 lines
fs/btrfs/inode.c +13
-0+ + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return trans; + ret =btrfs_cond_migrate_bytes (root->fs_info, + &root->fs_info->trans_block_rsv, + num_bytes,5 ); +if (ret) { +btrfs_end_transaction (trans, root); +return ERR_PTR (ret); } + trans->block_rsv = &root->fs_info->trans_block_rsv; + trans->bytes_reserved = num_bytes; +return trans; }
PATCH v3 1/1 btrfs: remove unnecessary error check
Hi Eric,
Quoted 50 lines · Eric Sandeen
Quoted 12 lines · Satoru Takeuchi
Quoted 11 lines · Eric Sandeen
Quoted 10 lines · Satoru Takeuchi
From: Satoru Takeuchi <[email protected]>
If "(!IS_ERR(trans) ||
PTR_ERR(trans)!=-ENOSPC))" is false, obviously "trans" is-ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans)==-ENOSPC)" check.Quoted 8 lines
Quoted 5 lines
True, but now a comment like:
/* Handle ENOSPC */
might still be nice...
Eric, thank you for your comment. I fixed my patch. How about is it?
One other thing I missed the first time, I'm sorry, notes below:
Quoted 26 lines
===
From: Satoru Takeuchi <[email protected]>If "(!IS_ERR(trans) ||
PTR_ERR(trans)!=-ENOSPC))" is false, obviously "trans" is-ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans)==-ENOSPC)" check.Signed-off-by Satoru Takeuchi <[email protected]>--- fs/btrfs/inode.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)fs/btrfs/inode.c +1
-2@@ -3803,22 +3803,23 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir)if (!IS_ERR (trans) ||PTR_ERR (trans) != -ENOSPC )return trans; -if (PTR_ERR (trans) == -ENOSPC ) { -u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); +/* Handle ENOSPC */
Quoted 16 lines
fs/btrfs/inode.c static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir) · line 3808
Quoted 13 lines
fs/btrfs/inode.c +1
-12- trans =btrfs_start_transaction (root,0 ); -if (IS_ERR (trans)) -return trans; - ret =btrfs_cond_migrate_bytes (root->fs_info, - &root->fs_info->trans_block_rsv, - num_bytes,5 ); -if (ret) { -btrfs_end_transaction (trans, root); -return ERR_PTR (ret); - } - trans->block_rsv = &root->fs_info->trans_block_rsv; - trans->bytes_reserved = num_bytes; +u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 );This variable should be declared at the beginning of the function, not in the middle, because it's no longer in a separate code block.
OK, moved.
Quoted 3 lines
Also, somehow by the time the patch got here, tabs turned into 4 spaces, so this one wouldn't apply for me.
I did't realize that. Thank you.
Quoted 2 lines
Sorry for missing the variable declaration problem the first time!
No problem, more review is welcome. THank you very much :-)
This is the v3 patch.
===
From: Satoru Takeuchi <[email protected]>
If "(!IS_ERR(trans) || PTR_ERR(trans) != -ENOSPC))" is false, obviously "trans" is -ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans) == -ENOSPC)" check.
--- fs/btrfs/inode.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)
fs/btrfs/inode.c +14 -14
@@ -3790,6 +3790,7 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir) {struct btrfs_trans_handle *trans;struct btrfs_root *root =BTRFS_I (dir)->root; +u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 );int ret;/*
@@ -3803,22 +3804,21 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir)if (!IS_ERR (trans) ||PTR_ERR (trans) != -ENOSPC )return trans; -if (PTR_ERR (trans) == -ENOSPC ) { -u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); +/* Handle ENOSPC */ - trans =btrfs_start_transaction (root,0 ); -if (IS_ERR (trans)) -return trans; - ret =btrfs_cond_migrate_bytes (root->fs_info, - &root->fs_info->trans_block_rsv, - num_bytes,5 ); -if (ret) { -btrfs_end_transaction (trans, root); -return ERR_PTR (ret); - } - trans->block_rsv = &root->fs_info->trans_block_rsv; - trans->bytes_reserved = num_bytes; + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return trans; + ret =btrfs_cond_migrate_bytes (root->fs_info, + &root->fs_info->trans_block_rsv, + num_bytes,5 ); +if (ret) { +btrfs_end_transaction (trans, root); +return ERR_PTR (ret); } + trans->block_rsv = &root->fs_info->trans_block_rsv; + trans->bytes_reserved = num_bytes; +return trans; }
Signature · 1 line
1.9.3
Quoted 10 lines · Satoru Takeuchi
This is the v3 patch.
===
From: Satoru Takeuchi <[email protected]>If "(!IS_ERR(trans) ||
PTR_ERR(trans)!=-ENOSPC))" is false, obviously "trans" is-ENOSPC. So we can safely remove the redundant "(PTR_ERR(trans)==-ENOSPC)" check.Signed-off-by Satoru Takeuchi <[email protected]>
Quoted 52 lines
--- fs/btrfs/inode.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)fs/btrfs/inode.c +14
-14@@ -3790,6 +3790,7 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir) {struct btrfs_trans_handle *trans;struct btrfs_root *root =BTRFS_I (dir)->root; +u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 );int ret;/* @@ -3803,22 +3804,21 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct inode *dir)if (!IS_ERR (trans) ||PTR_ERR (trans) != -ENOSPC )return trans; -if (PTR_ERR (trans) == -ENOSPC ) { -u64 num_bytes =btrfs_calc_trans_metadata_size (root,5 ); +/* Handle ENOSPC */ - trans =btrfs_start_transaction (root,0 ); -if (IS_ERR (trans)) -return trans; - ret =btrfs_cond_migrate_bytes (root->fs_info, - &root->fs_info->trans_block_rsv, - num_bytes,5 ); -if (ret) { -btrfs_end_transaction (trans, root); -return ERR_PTR (ret); - } - trans->block_rsv = &root->fs_info->trans_block_rsv; - trans->bytes_reserved = num_bytes; + trans =btrfs_start_transaction (root,0 ); +if (IS_ERR (trans)) +return trans; + ret =btrfs_cond_migrate_bytes (root->fs_info, + &root->fs_info->trans_block_rsv, + num_bytes,5 ); +if (ret) { +btrfs_end_transaction (trans, root); +return ERR_PTR (ret); } + trans->block_rsv = &root->fs_info->trans_block_rsv; + trans->bytes_reserved = num_bytes; +return trans; }