| Message ID | 1537950945-5627-1-git-send-email-zhongjiang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: list usage cleanup | expand |
On 26.09.2018 11:35, zhong jiang wrote: > Trival cleanup, list_move_tail will implement the same function that > list_del() + list_add_tail() will do. hence just replace them. > > Signed-off-by: zhong jiang <zhongjiang@huawei.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com> > --- > fs/btrfs/send.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index 094cc144..d87f416 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx, > */ > static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce) > { > - list_del(&nce->list); > - list_add_tail(&nce->list, &sctx->name_cache_list); > + list_move_tail(&nce->list, &sctx->name_cache_list); > } > > /* >
On Wed, Sep 26, 2018 at 04:35:45PM +0800, zhong jiang wrote: > Trival cleanup, list_move_tail will implement the same function that > list_del() + list_add_tail() will do. hence just replace them. Thanks, I've checked that this is the only instance where this cleanup is applicable. Reviewed-by: David Sterba <dsterba@suse.com>
On Wed, Sep 26, 2018 at 04:35:45PM +0800, zhong jiang wrote: > Trival cleanup, list_move_tail will implement the same function that > list_del() + list_add_tail() will do. hence just replace them. > > Signed-off-by: zhong jiang <zhongjiang@huawei.com> > --- > fs/btrfs/send.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index 094cc144..d87f416 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx, > */ > static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce) > { > - list_del(&nce->list); > - list_add_tail(&nce->list, &sctx->name_cache_list); > + list_move_tail(&nce->list, &sctx->name_cache_list); > } At that point do we even need such a trivial helper, considering that this is only called in one place?
On Thu, Sep 27, 2018 at 11:47:04AM -0700, Omar Sandoval wrote: > On Wed, Sep 26, 2018 at 04:35:45PM +0800, zhong jiang wrote: > > Trival cleanup, list_move_tail will implement the same function that > > list_del() + list_add_tail() will do. hence just replace them. > > > > Signed-off-by: zhong jiang <zhongjiang@huawei.com> > > --- > > fs/btrfs/send.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > > index 094cc144..d87f416 100644 > > --- a/fs/btrfs/send.c > > +++ b/fs/btrfs/send.c > > @@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx, > > */ > > static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce) > > { > > - list_del(&nce->list); > > - list_add_tail(&nce->list, &sctx->name_cache_list); > > + list_move_tail(&nce->list, &sctx->name_cache_list); > > } > > At that point do we even need such a trivial helper, considering that > this is only called in one place? Fair point and trivial one-line helpers are on the cleanup todo list. The exception is when the actual helper implementation is obscuring the semantics and the helper is used in many places so it's not practical to add a comment everywhere. But it's not the case here. Zhong Jiang, please update the patch and resend, thanks.
On 2018/10/2 0:48, David Sterba wrote: > On Thu, Sep 27, 2018 at 11:47:04AM -0700, Omar Sandoval wrote: >> On Wed, Sep 26, 2018 at 04:35:45PM +0800, zhong jiang wrote: >>> Trival cleanup, list_move_tail will implement the same function that >>> list_del() + list_add_tail() will do. hence just replace them. >>> >>> Signed-off-by: zhong jiang <zhongjiang@huawei.com> >>> --- >>> fs/btrfs/send.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c >>> index 094cc144..d87f416 100644 >>> --- a/fs/btrfs/send.c >>> +++ b/fs/btrfs/send.c >>> @@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx, >>> */ >>> static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce) >>> { >>> - list_del(&nce->list); >>> - list_add_tail(&nce->list, &sctx->name_cache_list); >>> + list_move_tail(&nce->list, &sctx->name_cache_list); >>> } >> At that point do we even need such a trivial helper, considering that >> this is only called in one place? > Fair point and trivial one-line helpers are on the cleanup todo list. > The exception is when the actual helper implementation is obscuring the > semantics and the helper is used in many places so it's not practical to > add a comment everywhere. But it's not the case here. > > Zhong Jiang, please update the patch and resend, thanks. Will add a comment here and resend in v2. Thanks, zhong jiang > . >
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 094cc144..d87f416 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -2075,8 +2075,7 @@ static struct name_cache_entry *name_cache_search(struct send_ctx *sctx, */ static void name_cache_used(struct send_ctx *sctx, struct name_cache_entry *nce) { - list_del(&nce->list); - list_add_tail(&nce->list, &sctx->name_cache_list); + list_move_tail(&nce->list, &sctx->name_cache_list); } /*
Trival cleanup, list_move_tail will implement the same function that list_del() + list_add_tail() will do. hence just replace them. Signed-off-by: zhong jiang <zhongjiang@huawei.com> --- fs/btrfs/send.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)