| Message ID | 20230907133928.11126-1-lhenriques@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | ceph: remove unnecessary check for NULL in parse_longname() | expand |
On 9/7/23 21:39, Luís Henriques wrote: > Function ceph_get_inode() never returns NULL; instead it returns an > ERR_PTR() if something fails. Thus, the check for NULL in > parse_longname() useless and can be dropped. > > Fixes: dd66df0053ef ("ceph: add support for encrypted snapshot names") > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Signed-off-by: Luís Henriques <lhenriques@suse.de> > --- > fs/ceph/crypto.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c > index e4d5cd56a80b..7d0b9b5ccfc6 100644 > --- a/fs/ceph/crypto.c > +++ b/fs/ceph/crypto.c > @@ -249,8 +249,6 @@ static struct inode *parse_longname(const struct inode *parent, > if (!dir) { > /* This can happen if we're not mounting cephfs on the root */ > dir = ceph_get_inode(parent->i_sb, vino, NULL); > - if (!dir) > - dir = ERR_PTR(-ENOENT); > } > if (IS_ERR(dir)) > dout("Can't find inode %s (%s)\n", inode_number, name); > Luis, How about moving the error check into the 'if (!dir) {}' ? Because from 'ceph_find_inode()' the return value shouldn't be true here. This err check should for 'ceph_get_inode()' only. Thanks - Xiubo
Xiubo Li <xiubli@redhat.com> writes: > On 9/7/23 21:39, Luís Henriques wrote: >> Function ceph_get_inode() never returns NULL; instead it returns an >> ERR_PTR() if something fails. Thus, the check for NULL in >> parse_longname() useless and can be dropped. >> >> Fixes: dd66df0053ef ("ceph: add support for encrypted snapshot names") >> Reported-by: Dan Carpenter <dan.carpenter@linaro.org> >> Signed-off-by: Luís Henriques <lhenriques@suse.de> >> --- >> fs/ceph/crypto.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c >> index e4d5cd56a80b..7d0b9b5ccfc6 100644 >> --- a/fs/ceph/crypto.c >> +++ b/fs/ceph/crypto.c >> @@ -249,8 +249,6 @@ static struct inode *parse_longname(const struct inode *parent, >> if (!dir) { >> /* This can happen if we're not mounting cephfs on the root */ >> dir = ceph_get_inode(parent->i_sb, vino, NULL); >> - if (!dir) >> - dir = ERR_PTR(-ENOENT); >> } >> if (IS_ERR(dir)) >> dout("Can't find inode %s (%s)\n", inode_number, name); >> > Luis, > > How about moving the error check into the 'if (!dir) {}' ? Because from > 'ceph_find_inode()' the return value shouldn't be true here. This err check > should for 'ceph_get_inode()' only. Yeah, you're right. Thanks, Xiubo. I'll send out v2 shortly. Cheers,
diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c index e4d5cd56a80b..7d0b9b5ccfc6 100644 --- a/fs/ceph/crypto.c +++ b/fs/ceph/crypto.c @@ -249,8 +249,6 @@ static struct inode *parse_longname(const struct inode *parent, if (!dir) { /* This can happen if we're not mounting cephfs on the root */ dir = ceph_get_inode(parent->i_sb, vino, NULL); - if (!dir) - dir = ERR_PTR(-ENOENT); } if (IS_ERR(dir)) dout("Can't find inode %s (%s)\n", inode_number, name);
Function ceph_get_inode() never returns NULL; instead it returns an ERR_PTR() if something fails. Thus, the check for NULL in parse_longname() useless and can be dropped. Fixes: dd66df0053ef ("ceph: add support for encrypted snapshot names") Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Luís Henriques <lhenriques@suse.de> --- fs/ceph/crypto.c | 2 -- 1 file changed, 2 deletions(-)