* [PATCH] ceph: initialize pathlen variable in reconnect_caps_cb
@ 2021-11-30 11:20 xiubli
2021-11-30 12:07 ` Jeff Layton
0 siblings, 1 reply; 5+ messages in thread
From: xiubli @ 2021-11-30 11:20 UTC (permalink / raw)
To: jlayton; +Cc: idryomov, vshankar, ukernel, ceph-devel, Xiubo Li
From: Xiubo Li <xiubli@redhat.com>
Silence the potential compiler warning.
Fixes: a33f6432b3a6 (ceph: encode inodes' parent/d_name in cap reconnect message)
Signed-off-by: Xiubo Li <xiubli@redhat.com>
---
fs/ceph/mds_client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 87f20ed16c6e..2fc2b0a023e4 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -3711,7 +3711,7 @@ static int reconnect_caps_cb(struct inode *inode, struct ceph_cap *cap,
struct ceph_pagelist *pagelist = recon_state->pagelist;
struct dentry *dentry;
char *path;
- int pathlen, err;
+ int pathlen = 0, err;
u64 pathbase;
u64 snap_follows;
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: initialize pathlen variable in reconnect_caps_cb
2021-11-30 11:20 [PATCH] ceph: initialize pathlen variable in reconnect_caps_cb xiubli
@ 2021-11-30 12:07 ` Jeff Layton
2021-11-30 13:12 ` Xiubo Li
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2021-11-30 12:07 UTC (permalink / raw)
To: xiubli; +Cc: idryomov, vshankar, ukernel, ceph-devel, dan.carpenter
On Tue, 2021-11-30 at 19:20 +0800, xiubli@redhat.com wrote:
> From: Xiubo Li <xiubli@redhat.com>
>
> Silence the potential compiler warning.
>
> Fixes: a33f6432b3a6 (ceph: encode inodes' parent/d_name in cap reconnect message)
> Signed-off-by: Xiubo Li <xiubli@redhat.com>
Is this something we need to fix? AFAICT, there is no bug here.
In the case where ceph_mdsc_build_path returns an error, "path" will be
an ERR_PTR and then ceph_mdsc_free_path will be a no-op. If we do need
to take this, we should probably also credit Dan for finding it.
> ---
> fs/ceph/mds_client.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> index 87f20ed16c6e..2fc2b0a023e4 100644
> --- a/fs/ceph/mds_client.c
> +++ b/fs/ceph/mds_client.c
> @@ -3711,7 +3711,7 @@ static int reconnect_caps_cb(struct inode *inode, struct ceph_cap *cap,
> struct ceph_pagelist *pagelist = recon_state->pagelist;
> struct dentry *dentry;
> char *path;
> - int pathlen, err;
> + int pathlen = 0, err;
> u64 pathbase;
> u64 snap_follows;
>
If we do take this, you can also get rid of the place where pathlen is
set in the !dentry case.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: initialize pathlen variable in reconnect_caps_cb
2021-11-30 12:07 ` Jeff Layton
@ 2021-11-30 13:12 ` Xiubo Li
2021-11-30 13:55 ` Jeff Layton
2021-11-30 14:35 ` Dan Carpenter
0 siblings, 2 replies; 5+ messages in thread
From: Xiubo Li @ 2021-11-30 13:12 UTC (permalink / raw)
To: Jeff Layton; +Cc: idryomov, vshankar, ukernel, ceph-devel, dan.carpenter
On 11/30/21 8:07 PM, Jeff Layton wrote:
> On Tue, 2021-11-30 at 19:20 +0800, xiubli@redhat.com wrote:
>> From: Xiubo Li <xiubli@redhat.com>
>>
>> Silence the potential compiler warning.
>>
>> Fixes: a33f6432b3a6 (ceph: encode inodes' parent/d_name in cap reconnect message)
>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> Is this something we need to fix? AFAICT, there is no bug here.
>
> In the case where ceph_mdsc_build_path returns an error, "path" will be
> an ERR_PTR and then ceph_mdsc_free_path will be a no-op. If we do need
> to take this, we should probably also credit Dan for finding it.
>
As I remembered, when I was paying the gluster-block project, the
similar cases will always give a warning like this with code sanity
checking.
>> ---
>> fs/ceph/mds_client.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
>> index 87f20ed16c6e..2fc2b0a023e4 100644
>> --- a/fs/ceph/mds_client.c
>> +++ b/fs/ceph/mds_client.c
>> @@ -3711,7 +3711,7 @@ static int reconnect_caps_cb(struct inode *inode, struct ceph_cap *cap,
>> struct ceph_pagelist *pagelist = recon_state->pagelist;
>> struct dentry *dentry;
>> char *path;
>> - int pathlen, err;
>> + int pathlen = 0, err;
>> u64 pathbase;
>> u64 snap_follows;
>>
> If we do take this, you can also get rid of the place where pathlen is
> set in the !dentry case.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: initialize pathlen variable in reconnect_caps_cb
2021-11-30 13:12 ` Xiubo Li
@ 2021-11-30 13:55 ` Jeff Layton
2021-11-30 14:35 ` Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: Jeff Layton @ 2021-11-30 13:55 UTC (permalink / raw)
To: Xiubo Li; +Cc: idryomov, vshankar, ukernel, ceph-devel, dan.carpenter
On Tue, 2021-11-30 at 21:12 +0800, Xiubo Li wrote:
> On 11/30/21 8:07 PM, Jeff Layton wrote:
> > On Tue, 2021-11-30 at 19:20 +0800, xiubli@redhat.com wrote:
> > > From: Xiubo Li <xiubli@redhat.com>
> > >
> > > Silence the potential compiler warning.
> > >
> > > Fixes: a33f6432b3a6 (ceph: encode inodes' parent/d_name in cap reconnect message)
> > > Signed-off-by: Xiubo Li <xiubli@redhat.com>
> > Is this something we need to fix? AFAICT, there is no bug here.
> >
> > In the case where ceph_mdsc_build_path returns an error, "path" will be
> > an ERR_PTR and then ceph_mdsc_free_path will be a no-op. If we do need
> > to take this, we should probably also credit Dan for finding it.
> >
> As I remembered, when I was paying the gluster-block project, the
> similar cases will always give a warning like this with code sanity
> checking.
>
Meh...ok.
I merged a slightly altered version of the patch into the testing
branch, revised the changelog and gave Dan reported-by credit. Please
take a look when you get a chance and let me know if anything is amiss.
Thanks,
> > > ---
> > > fs/ceph/mds_client.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
> > > index 87f20ed16c6e..2fc2b0a023e4 100644
> > > --- a/fs/ceph/mds_client.c
> > > +++ b/fs/ceph/mds_client.c
> > > @@ -3711,7 +3711,7 @@ static int reconnect_caps_cb(struct inode *inode, struct ceph_cap *cap,
> > > struct ceph_pagelist *pagelist = recon_state->pagelist;
> > > struct dentry *dentry;
> > > char *path;
> > > - int pathlen, err;
> > > + int pathlen = 0, err;
> > > u64 pathbase;
> > > u64 snap_follows;
> > >
> > If we do take this, you can also get rid of the place where pathlen is
> > set in the !dentry case.
> >
>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ceph: initialize pathlen variable in reconnect_caps_cb
2021-11-30 13:12 ` Xiubo Li
2021-11-30 13:55 ` Jeff Layton
@ 2021-11-30 14:35 ` Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2021-11-30 14:35 UTC (permalink / raw)
To: Xiubo Li; +Cc: Jeff Layton, idryomov, vshankar, ukernel, ceph-devel
On Tue, Nov 30, 2021 at 09:12:42PM +0800, Xiubo Li wrote:
>
> On 11/30/21 8:07 PM, Jeff Layton wrote:
> > On Tue, 2021-11-30 at 19:20 +0800, xiubli@redhat.com wrote:
> > > From: Xiubo Li <xiubli@redhat.com>
> > >
> > > Silence the potential compiler warning.
> > >
> > > Fixes: a33f6432b3a6 (ceph: encode inodes' parent/d_name in cap reconnect message)
> > > Signed-off-by: Xiubo Li <xiubli@redhat.com>
> > Is this something we need to fix? AFAICT, there is no bug here.
> >
> > In the case where ceph_mdsc_build_path returns an error, "path" will be
> > an ERR_PTR and then ceph_mdsc_free_path will be a no-op. If we do need
> > to take this, we should probably also credit Dan for finding it.
> >
> As I remembered, when I was paying the gluster-block project, the similar
> cases will always give a warning like this with code sanity checking.
>
I think I was just having a discussion about this. Do you remember
what the warnings look like?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-11-30 14:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30 11:20 [PATCH] ceph: initialize pathlen variable in reconnect_caps_cb xiubli
2021-11-30 12:07 ` Jeff Layton
2021-11-30 13:12 ` Xiubo Li
2021-11-30 13:55 ` Jeff Layton
2021-11-30 14:35 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.