sparc: vDSO: Silence an uninitialized variable warning
Subsystems (2)
PATCH 1/1 sparc: vDSO: Silence an uninitialized variable warning
Smatch complains that "val" would be uninitialized if kstrtoul() fails.
--- arch/sparc/vdso/vma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
arch/sparc/vdso/vma.c +3 -1
@@ -262,7 +262,9 @@ static __init int vdso_setup(char *s)unsigned long val; err =kstrtoul (s,10 , &val); +if (err) +return err; vdso_enabled = val; -return err; +return 0 ; }__setup ("vdso=" , vdso_setup);
Signature · 1 line
2.18.0
Quoted 14 lines · Dan Carpenter
Smatch complains that "val" would be uninitialized if
kstrtoul()fails.Fixes 9a08862a5d2e ("vDSO for sparc")Signed-off-by Dan Carpenter <[email protected]>--- arch/sparc/vdso/vma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)arch/sparc/vdso/vma.c +0
-0
arch/sparc/vdso/vma.c static __init int vdso_setup(char *s) · line 270
Quoted 10 lines
arch/sparc/vdso/vma.c +3
-1@@ -262,7 +262,9 @@ static __init int vdso_setup(char *s)unsigned long val; err =kstrtoul (s,10 , &val); +if (err) +return err; vdso_enabled = val; -return err; +return 0 ; }__setup ("vdso=" , vdso_setup);
This is probably fine, but it might be cleaner as
err = kstrtoul(s, 10, &val); if (!err) vdso_enabled = val; return err;
sln
Quoted 27 lines · Shannon Nelson
Quoted 13 lines · Dan Carpenter
Smatch complains that "val" would be uninitialized if
kstrtoul()fails.Fixes 9a08862a5d2e ("vDSO for sparc")Signed-off-by Dan Carpenter <[email protected]>--- arch/sparc/vdso/vma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)arch/sparc/vdso/vma.c +0
-0arch/sparc/vdso/vma.c static __init int vdso_setup(char *s) · line 270
Quoted 10 lines
arch/sparc/vdso/vma.c +3
-1@@ -262,7 +262,9 @@ static __init int vdso_setup(char *s)unsigned long val; err =kstrtoul (s,10 , &val); +if (err) +return err; vdso_enabled = val; -return err; +return 0 ; }__setup ("vdso=" , vdso_setup);This is probably fine, but it might be cleaner as
Quoted 4 lines
err = kstrtoul(s, 10, &val); if (!err) vdso_enabled = val; return err;
I always tell people to do failure handling instead of success handling. And also I tell people not to make the last if statement in a function a special case. Like you see this a lot right:
ret = frob(); if (ret) return ret; ret = frob(); if (ret) return ret; ret = frob(); if (!ret) *p = whatever; return ret;
If you do failure handling then the success path is at indent level one and failure handling is two tabs. But if you mix it up it's not as clear. I have never studied this, but my impression from looking at static analysis over the years is that success handling is error prone.
regards,
dan carpenter
Quoted 50 lines · Dan Carpenter
Quoted 27 lines · Shannon Nelson
Quoted 13 lines · Dan Carpenter
Smatch complains that "val" would be uninitialized if
kstrtoul()fails.Fixes 9a08862a5d2e ("vDSO for sparc")Signed-off-by Dan Carpenter <[email protected]>--- arch/sparc/vdso/vma.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)arch/sparc/vdso/vma.c +0
-0arch/sparc/vdso/vma.c static __init int vdso_setup(char *s) · line 270
Quoted 10 lines
arch/sparc/vdso/vma.c +3
-1@@ -262,7 +262,9 @@ static __init int vdso_setup(char *s)unsigned long val; err =kstrtoul (s,10 , &val); +if (err) +return err; vdso_enabled = val; -return err; +return 0 ; }__setup ("vdso=" , vdso_setup);This is probably fine, but it might be cleaner as
Quoted 4 lines
err = kstrtoul(s, 10, &val); if (!err) vdso_enabled = val; return err;I always tell people to do failure handling instead of success handling. And also I tell people not to make the last if statement in a function a special case. Like you see this a lot right:
ret = frob(); if (ret) return ret; ret = frob(); if (ret) return ret; ret = frob(); if (!ret) *p = whatever; return ret;
Quoted 8 lines
If you do failure handling then the success path is at indent level one and failure handling is two tabs. But if you mix it up it's not as clear. I have never studied this, but my impression from looking at static analysis over the years is that success handling is error prone.
regards,
dan carpenter
(driving right into the bikeshed...)
Yes, and that's the way I would normally go, especially if there was more to that little function. But in this case with such a short little function, I hate seeing it messed up with unnecessary lines.
Anyway, enough with the noise, either way is fine.
sln
From: Dan Carpenter <[email protected]>
Date: Sat, 13 Oct 2018 13:26:53 +0300
Quoted 4 lines
Smatch complains that "val" would be uninitialized if
kstrtoul()fails.Fixes 9a08862a5d2e ("vDSO for sparc")Signed-off-by Dan Carpenter <[email protected]>
So much bike shedding in this thread ;-)
Applied, thanks Dan.