From: Colin Ian King <[email protected]>
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 <[email protected]>
---
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
On 07/08/2019 13:35, Joseph Qi wrote:
>
>
> On 19/8/7 20:19, Colin King wrote:
>> From: Colin Ian King <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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)
>>
On 19/8/7 20:19, Colin King wrote:
> From: Colin Ian King <[email protected]>
>
> 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 <[email protected]>
> ---
> 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)
>
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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>
>>>
>>> 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