| Message ID | 3034f66918cf06066b769f4d834905e25824ae8c.1719336718.git.boris@bur.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: don't check NULL ulist in ulist_prealloc | expand |
On Tue, Jun 25, 2024 at 6:35 PM Boris Burkov <boris@bur.io> wrote: > > There are no callers with a NULL ulist, so the check is not meaningful. > > Signed-off-by: Boris Burkov <boris@bur.io> Reviewed-by: Filipe Manana <fdmanana@suse.com> Looks, good thanks. Can be squashed into the patch that introduced this function. > --- > fs/btrfs/ulist.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c > index f35d3e93996b..fc59b57257d6 100644 > --- a/fs/btrfs/ulist.c > +++ b/fs/btrfs/ulist.c > @@ -110,7 +110,7 @@ struct ulist *ulist_alloc(gfp_t gfp_mask) > > void ulist_prealloc(struct ulist *ulist, gfp_t gfp_mask) > { > - if (ulist && !ulist->prealloc) > + if (!ulist->prealloc) > ulist->prealloc = kzalloc(sizeof(*ulist->prealloc), gfp_mask); > } > > -- > 2.45.2 > >
On Tue, Jun 25, 2024 at 06:45:01PM +0100, Filipe Manana wrote: > On Tue, Jun 25, 2024 at 6:35 PM Boris Burkov <boris@bur.io> wrote: > > > > There are no callers with a NULL ulist, so the check is not meaningful. > > > > Signed-off-by: Boris Burkov <boris@bur.io> > > Reviewed-by: Filipe Manana <fdmanana@suse.com> > > Looks, good thanks. > Can be squashed into the patch that introduced this function. > Done, thanks. > > --- > > fs/btrfs/ulist.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c > > index f35d3e93996b..fc59b57257d6 100644 > > --- a/fs/btrfs/ulist.c > > +++ b/fs/btrfs/ulist.c > > @@ -110,7 +110,7 @@ struct ulist *ulist_alloc(gfp_t gfp_mask) > > > > void ulist_prealloc(struct ulist *ulist, gfp_t gfp_mask) > > { > > - if (ulist && !ulist->prealloc) > > + if (!ulist->prealloc) > > ulist->prealloc = kzalloc(sizeof(*ulist->prealloc), gfp_mask); > > } > > > > -- > > 2.45.2 > > > >
On Tue, Jun 25, 2024 at 10:32:11AM -0700, Boris Burkov wrote: > There are no callers with a NULL ulist, so the check is not meaningful. > > Signed-off-by: Boris Burkov <boris@bur.io> > --- > fs/btrfs/ulist.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c > index f35d3e93996b..fc59b57257d6 100644 > --- a/fs/btrfs/ulist.c > +++ b/fs/btrfs/ulist.c > @@ -110,7 +110,7 @@ struct ulist *ulist_alloc(gfp_t gfp_mask) > > void ulist_prealloc(struct ulist *ulist, gfp_t gfp_mask) > { > - if (ulist && !ulist->prealloc) > + if (!ulist->prealloc) You can also add an assert so it's visible that it's the assumption. > ulist->prealloc = kzalloc(sizeof(*ulist->prealloc), gfp_mask); > } > > -- > 2.45.2 >
On Wed, Jun 26, 2024 at 03:22:57PM +0200, David Sterba wrote: > On Tue, Jun 25, 2024 at 10:32:11AM -0700, Boris Burkov wrote: > > There are no callers with a NULL ulist, so the check is not meaningful. > > > > Signed-off-by: Boris Burkov <boris@bur.io> > > --- > > fs/btrfs/ulist.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c > > index f35d3e93996b..fc59b57257d6 100644 > > --- a/fs/btrfs/ulist.c > > +++ b/fs/btrfs/ulist.c > > @@ -110,7 +110,7 @@ struct ulist *ulist_alloc(gfp_t gfp_mask) > > > > void ulist_prealloc(struct ulist *ulist, gfp_t gfp_mask) > > { > > - if (ulist && !ulist->prealloc) > > + if (!ulist->prealloc) > > You can also add an assert so it's visible that it's the assumption. Reading the previous thread where this was discussed, for interfaces that are self-contained the assertions are meant as a documentation of the API but it's not done consistently. Without the assertion is fine for me as well.
diff --git a/fs/btrfs/ulist.c b/fs/btrfs/ulist.c index f35d3e93996b..fc59b57257d6 100644 --- a/fs/btrfs/ulist.c +++ b/fs/btrfs/ulist.c @@ -110,7 +110,7 @@ struct ulist *ulist_alloc(gfp_t gfp_mask) void ulist_prealloc(struct ulist *ulist, gfp_t gfp_mask) { - if (ulist && !ulist->prealloc) + if (!ulist->prealloc) ulist->prealloc = kzalloc(sizeof(*ulist->prealloc), gfp_mask); }
There are no callers with a NULL ulist, so the check is not meaningful. Signed-off-by: Boris Burkov <boris@bur.io> --- fs/btrfs/ulist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)