Re: [PATCH] ceph: initialize pathlen variable in reconnect_caps_cb

From: Jeff Layton <jlayton@kernel.org>
To: Xiubo Li <xiubli@redhat.com>
Cc: idryomov@gmail.com, vshankar@redhat.com, ukernel@gmail.com,
	ceph-devel@vger.kernel.org, dan.carpenter@oracle.com
Subject: Re: [PATCH] ceph: initialize pathlen variable in reconnect_caps_cb
Date: Tue, 30 Nov 2021 08:55:22 -0500	[thread overview]
Message-ID: <7965c4707d595fd488eeed61731d5fc336c75e63.camel@kernel.org> (raw)
In-Reply-To: <d14840f4-3ad8-55ce-480c-4d8cf3234893@redhat.com>

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>