[PATCH][ocfs2-next] ocfs2: ensure ret is set to zero before returning

[PATCH][ocfs2-next] ocfs2: ensure ret is set to zero before returning

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

A previous commit introduced a regression where variable ret was
originally being set from the return from a call to function
dlm_create_debugfs_subroot and this set was removed. Currently
ret is now uninitialized if no alloction errors are found which
may end up with a bogus check on ret < 0 on the 'leave:' return
path.  Fix this by setting ret to zero on a successful execution
path.

Addresses-Coverity: ("Uninitialzed scalar variable")
Fixes: cba322160ef0 ("ocfs2: further debugfs cleanups")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 fs/ocfs2/dlm/dlmdomain.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
index 5c4218d66dd2..ee6f459f9770 100644
--- a/fs/ocfs2/dlm/dlmdomain.c
+++ b/fs/ocfs2/dlm/dlmdomain.c
@@ -2052,6 +2052,7 @@ static struct dlm_ctxt *dlm_alloc_ctxt(const char *domain,
 	mlog(0, "context init: refcount %u\n",
 		  kref_read(&dlm->dlm_refs));
 
+	ret = 0;
 leave:
 	if (ret < 0 && dlm) {
 		if (dlm->master_hash)
-- 
2.20.1

Re: [PATCH][ocfs2-next] ocfs2: ensure ret is set to zero before returning

[Joseph Qi]

On 19/8/7 20:19, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> A previous commit introduced a regression where variable ret was
> originally being set from the return from a call to function
> dlm_create_debugfs_subroot and this set was removed. Currently
> ret is now uninitialized if no alloction errors are found which
> may end up with a bogus check on ret < 0 on the 'leave:' return
> path.  Fix this by setting ret to zero on a successful execution
> path.

Good catch.
Or shall we just initialize 'ret' at first?

> 
> Addresses-Coverity: ("Uninitialzed scalar variable")

Typo here. 

Thanks,
Joseph

> Fixes: cba322160ef0 ("ocfs2: further debugfs cleanups")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  fs/ocfs2/dlm/dlmdomain.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
> index 5c4218d66dd2..ee6f459f9770 100644
> --- a/fs/ocfs2/dlm/dlmdomain.c
> +++ b/fs/ocfs2/dlm/dlmdomain.c
> @@ -2052,6 +2052,7 @@ static struct dlm_ctxt *dlm_alloc_ctxt(const char *domain,
>  	mlog(0, "context init: refcount %u\n",
>  		  kref_read(&dlm->dlm_refs));
>  
> +	ret = 0;
>  leave:
>  	if (ret < 0 && dlm) {
>  		if (dlm->master_hash)
> 

Re: [PATCH][ocfs2-next] ocfs2: ensure ret is set to zero before returning

[Colin Ian King]

On 07/08/2019 13:35, Joseph Qi wrote:
> 
> 
> On 19/8/7 20:19, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> A previous commit introduced a regression where variable ret was
>> originally being set from the return from a call to function
>> dlm_create_debugfs_subroot and this set was removed. Currently
>> ret is now uninitialized if no alloction errors are found which
>> may end up with a bogus check on ret < 0 on the 'leave:' return
>> path.  Fix this by setting ret to zero on a successful execution
>> path.
> 
> Good catch.
> Or shall we just initialize 'ret' at first?

Initialized ret first may not catch subsequent coding errors where error
returns paths have not initialized ret, so my preference is when it is
required and not before.

> 
>>
>> Addresses-Coverity: ("Uninitialzed scalar variable")

Can this be fixed up when applied rather sending a V2?
> 
> Typo here. 
> 
> Thanks,
> Joseph
> 
>> Fixes: cba322160ef0 ("ocfs2: further debugfs cleanups")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  fs/ocfs2/dlm/dlmdomain.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c
>> index 5c4218d66dd2..ee6f459f9770 100644
>> --- a/fs/ocfs2/dlm/dlmdomain.c
>> +++ b/fs/ocfs2/dlm/dlmdomain.c
>> @@ -2052,6 +2052,7 @@ static struct dlm_ctxt *dlm_alloc_ctxt(const char *domain,
>>  	mlog(0, "context init: refcount %u\n",
>>  		  kref_read(&dlm->dlm_refs));
>>  
>> +	ret = 0;
>>  leave:
>>  	if (ret < 0 && dlm) {
>>  		if (dlm->master_hash)
>>

Re: [PATCH][ocfs2-next] ocfs2: ensure ret is set to zero before returning

[Joseph Qi]

On 19/8/7 20:42, Colin Ian King wrote:
> On 07/08/2019 13:35, Joseph Qi wrote:
>>
>>
>> On 19/8/7 20:19, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> A previous commit introduced a regression where variable ret was
>>> originally being set from the return from a call to function
>>> dlm_create_debugfs_subroot and this set was removed. Currently
>>> ret is now uninitialized if no alloction errors are found which
>>> may end up with a bogus check on ret < 0 on the 'leave:' return
>>> path.  Fix this by setting ret to zero on a successful execution
>>> path.
>>
>> Good catch.
>> Or shall we just initialize 'ret' at first?
> 
> Initialized ret first may not catch subsequent coding errors where error
> returns paths have not initialized ret, so my preference is when it is
> required and not before.
> 
Okay, looks good to me.
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>

>>
>>>
>>> Addresses-Coverity: ("Uninitialzed scalar variable")
> 
> Can this be fixed up when applied rather sending a V2?
>>
Currently ocfs2 patches are maintained in Andrew's mm tree.
So it depends on Andrew.

Thanks,
Joseph