* [PATCH] ubi: wl: Silence uninitialized variable warning
@ 2019-02-28 5:35 Dan Carpenter
2019-02-28 8:35 ` Richard Weinberger
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2019-02-28 5:35 UTC (permalink / raw)
To: Artem Bityutskiy, Richard Weinberger
Cc: Boris Brezillon, kernel-janitors, Marek Vasut, linux-mtd,
Brian Norris, David Woodhouse
This condition needs to be fipped around because "err" is uninitialized
when "force" is set. The Smatch static analysis tool complains and
UBsan will also complain at runtime.
Fixes: 663586c0a892 ("ubi: Expose the bitrot interface")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/mtd/ubi/wl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 40f838d54b0f..2709dc02fc24 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -1517,7 +1517,7 @@ int ubi_bitflip_check(struct ubi_device *ubi, int pnum, int force)
mutex_unlock(&ubi->buf_mutex);
}
- if (err == UBI_IO_BITFLIPS || force) {
+ if (force || err == UBI_IO_BITFLIPS) {
/*
* Okay, bit flip happened, let's figure out what we can do.
*/
--
2.17.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ubi: wl: Silence uninitialized variable warning 2019-02-28 5:35 [PATCH] ubi: wl: Silence uninitialized variable warning Dan Carpenter @ 2019-02-28 8:35 ` Richard Weinberger 2019-02-28 8:50 ` Nathan Chancellor 0 siblings, 1 reply; 5+ messages in thread From: Richard Weinberger @ 2019-02-28 8:35 UTC (permalink / raw) To: Dan Carpenter Cc: Boris Brezillon, Artem Bityutskiy, kernel-janitors, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Am Donnerstag, 28. Februar 2019, 06:35:51 CET schrieb Dan Carpenter: > This condition needs to be fipped around because "err" is uninitialized > when "force" is set. The Smatch static analysis tool complains and > UBsan will also complain at runtime. > > Fixes: 663586c0a892 ("ubi: Expose the bitrot interface") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/mtd/ubi/wl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c > index 40f838d54b0f..2709dc02fc24 100644 > --- a/drivers/mtd/ubi/wl.c > +++ b/drivers/mtd/ubi/wl.c > @@ -1517,7 +1517,7 @@ int ubi_bitflip_check(struct ubi_device *ubi, int pnum, int force) > mutex_unlock(&ubi->buf_mutex); > } > > - if (err == UBI_IO_BITFLIPS || force) { > + if (force || err == UBI_IO_BITFLIPS) { > /* > * Okay, bit flip happened, let's figure out what we can do. > */ > Good catch, Dan! I thought gcc is supposed to find such issues too. :-/ Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ubi: wl: Silence uninitialized variable warning 2019-02-28 8:35 ` Richard Weinberger @ 2019-02-28 8:50 ` Nathan Chancellor 2019-02-28 9:51 ` Richard Weinberger 0 siblings, 1 reply; 5+ messages in thread From: Nathan Chancellor @ 2019-02-28 8:50 UTC (permalink / raw) To: Richard Weinberger Cc: Boris Brezillon, Artem Bityutskiy, kernel-janitors, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse, Dan Carpenter On Thu, Feb 28, 2019 at 09:35:50AM +0100, Richard Weinberger wrote: > Am Donnerstag, 28. Februar 2019, 06:35:51 CET schrieb Dan Carpenter: > > This condition needs to be fipped around because "err" is uninitialized > > when "force" is set. The Smatch static analysis tool complains and > > UBsan will also complain at runtime. > > > > Fixes: 663586c0a892 ("ubi: Expose the bitrot interface") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> Tested-by: Nathan Chancellor <natechancellor@gmail.com> This fixes a -Wsometimes-uninitialized warning from Clang: drivers/mtd/ubi/wl.c:1514:6: warning: variable 'err' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] if (!force) { ^~~~~~ drivers/mtd/ubi/wl.c:1520:6: note: uninitialized use occurs here if (err == UBI_IO_BITFLIPS || force) { ^~~ drivers/mtd/ubi/wl.c:1514:2: note: remove the 'if' if its condition is always true if (!force) { ^~~~~~~~~~~~ drivers/mtd/ubi/wl.c:1478:9: note: initialize the variable 'err' to silence this warning int err; ^ = 0 1 warning generated. > > --- > > drivers/mtd/ubi/wl.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c > > index 40f838d54b0f..2709dc02fc24 100644 > > --- a/drivers/mtd/ubi/wl.c > > +++ b/drivers/mtd/ubi/wl.c > > @@ -1517,7 +1517,7 @@ int ubi_bitflip_check(struct ubi_device *ubi, int pnum, int force) > > mutex_unlock(&ubi->buf_mutex); > > } > > > > - if (err == UBI_IO_BITFLIPS || force) { > > + if (force || err == UBI_IO_BITFLIPS) { > > /* > > * Okay, bit flip happened, let's figure out what we can do. > > */ > > > > Good catch, Dan! > I thought gcc is supposed to find such issues too. :-/ This isn't the first time GCC hasn't caught something... https://lore.kernel.org/lkml/20190221222123.GC6474@magnolia/ > > Thanks, > //richard > > Thanks, Nathan ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ubi: wl: Silence uninitialized variable warning 2019-02-28 8:50 ` Nathan Chancellor @ 2019-02-28 9:51 ` Richard Weinberger 2019-02-28 15:33 ` Nathan Chancellor 0 siblings, 1 reply; 5+ messages in thread From: Richard Weinberger @ 2019-02-28 9:51 UTC (permalink / raw) To: Nathan Chancellor Cc: Boris Brezillon, Artem Bityutskiy, kernel-janitors, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse, Dan Carpenter Am Donnerstag, 28. Februar 2019, 09:50:58 CET schrieb Nathan Chancellor: > On Thu, Feb 28, 2019 at 09:35:50AM +0100, Richard Weinberger wrote: > > Am Donnerstag, 28. Februar 2019, 06:35:51 CET schrieb Dan Carpenter: > > > This condition needs to be fipped around because "err" is uninitialized > > > when "force" is set. The Smatch static analysis tool complains and > > > UBsan will also complain at runtime. > > > > > > Fixes: 663586c0a892 ("ubi: Expose the bitrot interface") > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > Tested-by: Nathan Chancellor <natechancellor@gmail.com> Did you really test the code or just compile it? > This fixes a -Wsometimes-uninitialized warning from Clang: > > drivers/mtd/ubi/wl.c:1514:6: warning: variable 'err' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] > if (!force) { > ^~~~~~ > drivers/mtd/ubi/wl.c:1520:6: note: uninitialized use occurs here > if (err == UBI_IO_BITFLIPS || force) { > ^~~ > drivers/mtd/ubi/wl.c:1514:2: note: remove the 'if' if its condition is always true > if (!force) { > ^~~~~~~~~~~~ > drivers/mtd/ubi/wl.c:1478:9: note: initialize the variable 'err' to silence this warning > int err; > ^ > = 0 > 1 warning generated. How much false positives does this trigger? Many useful gcc warnings are disabled because they produce too much churn. > > > --- > > > drivers/mtd/ubi/wl.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c > > > index 40f838d54b0f..2709dc02fc24 100644 > > > --- a/drivers/mtd/ubi/wl.c > > > +++ b/drivers/mtd/ubi/wl.c > > > @@ -1517,7 +1517,7 @@ int ubi_bitflip_check(struct ubi_device *ubi, int pnum, int force) > > > mutex_unlock(&ubi->buf_mutex); > > > } > > > > > > - if (err == UBI_IO_BITFLIPS || force) { > > > + if (force || err == UBI_IO_BITFLIPS) { > > > /* > > > * Okay, bit flip happened, let's figure out what we can do. > > > */ > > > > > > > Good catch, Dan! > > I thought gcc is supposed to find such issues too. :-/ > > This isn't the first time GCC hasn't caught something... > > https://lore.kernel.org/lkml/20190221222123.GC6474@magnolia/ Compilers are not perfect. :-) Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ubi: wl: Silence uninitialized variable warning 2019-02-28 9:51 ` Richard Weinberger @ 2019-02-28 15:33 ` Nathan Chancellor 0 siblings, 0 replies; 5+ messages in thread From: Nathan Chancellor @ 2019-02-28 15:33 UTC (permalink / raw) To: Richard Weinberger Cc: Boris Brezillon, Artem Bityutskiy, kernel-janitors, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse, Dan Carpenter On Thu, Feb 28, 2019 at 10:51:12AM +0100, Richard Weinberger wrote: > Am Donnerstag, 28. Februar 2019, 09:50:58 CET schrieb Nathan Chancellor: > > On Thu, Feb 28, 2019 at 09:35:50AM +0100, Richard Weinberger wrote: > > > Am Donnerstag, 28. Februar 2019, 06:35:51 CET schrieb Dan Carpenter: > > > > This condition needs to be fipped around because "err" is uninitialized > > > > when "force" is set. The Smatch static analysis tool complains and > > > > UBsan will also complain at runtime. > > > > > > > > Fixes: 663586c0a892 ("ubi: Expose the bitrot interface") > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > > Tested-by: Nathan Chancellor <natechancellor@gmail.com> > > Did you really test the code or just compile it? > Compiled tested. If I shouldn't use that tag in that instance, please let me know. > > This fixes a -Wsometimes-uninitialized warning from Clang: > > > > drivers/mtd/ubi/wl.c:1514:6: warning: variable 'err' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] > > if (!force) { > > ^~~~~~ > > drivers/mtd/ubi/wl.c:1520:6: note: uninitialized use occurs here > > if (err == UBI_IO_BITFLIPS || force) { > > ^~~ > > drivers/mtd/ubi/wl.c:1514:2: note: remove the 'if' if its condition is always true > > if (!force) { > > ^~~~~~~~~~~~ > > drivers/mtd/ubi/wl.c:1478:9: note: initialize the variable 'err' to silence this warning > > int err; > > ^ > > = 0 > > 1 warning generated. > > How much false positives does this trigger? > Many useful gcc warnings are disabled because they produce too much churn. > I haven't gone through them all yet but it doesn't seem to trigger as often as GCC. I think there are only 22 files with a problem across arm, arm64, and x86_64 allyesconfig. > > > > --- > > > > drivers/mtd/ubi/wl.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c > > > > index 40f838d54b0f..2709dc02fc24 100644 > > > > --- a/drivers/mtd/ubi/wl.c > > > > +++ b/drivers/mtd/ubi/wl.c > > > > @@ -1517,7 +1517,7 @@ int ubi_bitflip_check(struct ubi_device *ubi, int pnum, int force) > > > > mutex_unlock(&ubi->buf_mutex); > > > > } > > > > > > > > - if (err == UBI_IO_BITFLIPS || force) { > > > > + if (force || err == UBI_IO_BITFLIPS) { > > > > /* > > > > * Okay, bit flip happened, let's figure out what we can do. > > > > */ > > > > > > > > > > Good catch, Dan! > > > I thought gcc is supposed to find such issues too. :-/ > > > > This isn't the first time GCC hasn't caught something... > > > > https://lore.kernel.org/lkml/20190221222123.GC6474@magnolia/ > > Compilers are not perfect. :-) Would make everyone's life a lot better if they were :) Nathan > > Thanks, > //richard > > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-28 15:34 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-02-28 5:35 [PATCH] ubi: wl: Silence uninitialized variable warning Dan Carpenter 2019-02-28 8:35 ` Richard Weinberger 2019-02-28 8:50 ` Nathan Chancellor 2019-02-28 9:51 ` Richard Weinberger 2019-02-28 15:33 ` Nathan Chancellor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox