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

From: Xiubo Li <xiubli@redhat.com>
To: Jeff Layton <jlayton@kernel.org>
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 21:12:42 +0800	[thread overview]
Message-ID: <d14840f4-3ad8-55ce-480c-4d8cf3234893@redhat.com> (raw)
In-Reply-To: <41b05af2020a3cb345a16f5dfca15f6f5f41bfe4.camel@kernel.org>


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.
>