* Re: [PATCH] btrfs: remove unneeded variable: "ret"
2021-06-28 8:30 [PATCH] btrfs: remove unneeded variable: "ret" 13145886936
@ 2021-06-28 9:21 ` Qu Wenruo
2021-06-28 11:38 ` Qu Wenruo
1 sibling, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2021-06-28 9:21 UTC (permalink / raw)
To: 13145886936, clm, josef, dsterba; +Cc: linux-btrfs, linux-kernel, gushengxian
On 2021/6/28 下午4:30, 13145886936@163.com wrote:
> From: gushengxian <gushengxian@yulong.com>
>
> Remove unneeded variable: "ret".
>
> Signed-off-by: gushengxian <13145886936@163.com>
> Signed-off-by: gushengxian <gushengxian@yulong.com>
Is this detected by some script?
Mind to share the script and run it against the whole btrfs code base?
Thanks,
Qu
> ---
> fs/btrfs/disk-io.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index b117dd3b8172..7e65a54b7839 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4624,7 +4624,6 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
> struct rb_node *node;
> struct btrfs_delayed_ref_root *delayed_refs;
> struct btrfs_delayed_ref_node *ref;
> - int ret = 0;
>
> delayed_refs = &trans->delayed_refs;
>
> @@ -4632,7 +4631,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
> if (atomic_read(&delayed_refs->num_entries) == 0) {
> spin_unlock(&delayed_refs->lock);
> btrfs_debug(fs_info, "delayed_refs has NO entry");
> - return ret;
> + return 0;
> }
>
> while ((node = rb_first_cached(&delayed_refs->href_root)) != NULL) {
> @@ -4695,7 +4694,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>
> spin_unlock(&delayed_refs->lock);
>
> - return ret;
> + return 0;
> }
>
> static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root)
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] btrfs: remove unneeded variable: "ret"
2021-06-28 8:30 [PATCH] btrfs: remove unneeded variable: "ret" 13145886936
2021-06-28 9:21 ` Qu Wenruo
@ 2021-06-28 11:38 ` Qu Wenruo
1 sibling, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2021-06-28 11:38 UTC (permalink / raw)
To: 13145886936, linux-kernel, linux-btrfs, gushengxian
Hi Gu,
On 2021/6/28 下午4:30, 13145886936@163.com wrote:
> From: gushengxian <gushengxian@yulong.com>
>
> Remove unneeded variable: "ret".
Considering you're really a newbie doing new contribution to the kernel,
I think it's better to give you some advice to encourage you do more
contribution, while also be more professional, and hopefully to help
other individuals in your company to contribute.
And I hope this could also be some guide for new developers to learn a
trick or two from.
=== WHEN NOT SURE, LEARN FROM OTHERS ===
First thing first, if you have something not sure, like how you should
setup your name and email in your git config, try to find some merged
patches to learn from them.
=== EMAIL ADDRESS ===
Firstly, your mail is sent from your 163 mail box, not your company mail
box.
Peter Zijlstra has already mentioned this in another patch.
Normally if you want to send a patch to represent your company, it's
common to send using a mail address of your company "@yulong.com" (Isn't
it called Coolpad Group nowadays?)
For how to configure git-send-email to use the SMTP server of your
company, I guess your colleague Hu Yue <huyue2@yulong.com> has more
experience and you can definitely learn from him.
If you want to the patch to be CCed to your personal mail box, you can
use "--cc" option of git-send-email, as most reviewer just reply-to-all,
thus your personal mail box will definite get the comment.
This will make the SOB line much cleaner.
=== MAIL LIST ===
This is not a big deal, just something optional but really helpful for
your next contribution.
In this particular patch, you only need to send the patch to btrfs mail
list <linux-btrfs@vger.kernel.org>, even no need to CC the maintainers.
LKML is fine for your first several patches to get more comments, like
this one.
But when you get settled to a certain field of kernel, it's better just
to send the patch to the related mail list.
=== FOR THE CLEANUP ===
As I mentioned in another thread, if you use some automatic tool or
script to expose the problem, that's fine.
But it would be even better to provide the tool. Fix one small problem
is OK, but fixing a type of problems is really what we want.
If you really just found the bug by manually scanning the code, kudos to
you.
But if you do more contribution, one day your time will be too precious
to be spent on things like this.
Just like an old Chinese saying, give a man a fish, and you feed him for
a day, teach a man to finish, and you feed him for a lifetime.
And since the patch itself is fine.
Reviewed-by: Qu Wenruo <wqu@suse.com>
And really hope you can do more and better contribution to linux kernel.
Thanks,
Qu
>
> Signed-off-by: gushengxian <13145886936@163.com>
> Signed-off-by: gushengxian <gushengxian@yulong.com>
> ---
> fs/btrfs/disk-io.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index b117dd3b8172..7e65a54b7839 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4624,7 +4624,6 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
> struct rb_node *node;
> struct btrfs_delayed_ref_root *delayed_refs;
> struct btrfs_delayed_ref_node *ref;
> - int ret = 0;
>
> delayed_refs = &trans->delayed_refs;
>
> @@ -4632,7 +4631,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
> if (atomic_read(&delayed_refs->num_entries) == 0) {
> spin_unlock(&delayed_refs->lock);
> btrfs_debug(fs_info, "delayed_refs has NO entry");
> - return ret;
> + return 0;
> }
>
> while ((node = rb_first_cached(&delayed_refs->href_root)) != NULL) {
> @@ -4695,7 +4694,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>
> spin_unlock(&delayed_refs->lock);
>
> - return ret;
> + return 0;
> }
>
> static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root)
>
^ permalink raw reply [flat|nested] 3+ messages in thread