Re: staging: most: warning: ‘mbo’ may be used uninitialized in this function
On Fri, Mar 18, 2016 at 01:41:19PM +0100, Geert Uytterhoeven wrote: > > @@ -249,11 +246,7 @@ aim_read(struct file *filp, char __user *buf, size_t > > count, loff_t *offset) > > struct aim_channel *c = filp->private_data; > > > > mutex_lock(>io_mutex); > > - if (c->stacked_mbo) { > > - mbo = c->stacked_mbo; > > - goto start_copy; > > - } > > - while ((!kfifo_out(>fifo, , 1)) && (c->dev)) { > > + while (c->dev && !kfifo_peek(>fifo, )) { > > drivers/staging/most/aim-cdev/cdev.c:241: warning: ‘mbo’ may be used > uninitialized in this function > > From looking at the code, it's not obvious to me if this is a false > positive or not. > Can it happen that mbo is not initialized fully, e.g. if less than sizeof(mbo) > bytes have been read from the kfifo? > > Other callers initialize the pointer to NULL, and check the returned length. > It looks like a false positive to me. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
staging: most: warning: ‘mbo’ may be used uninitialized in this function
On Fri, Mar 18, 2016 at 6:42 AM, Linux Kernel Mailing Listwrote: > Web: > https://git.kernel.org/torvalds/c/f45b0fba43f415f69982df743dfa9b5d1b57785e > Commit: f45b0fba43f415f69982df743dfa9b5d1b57785e > Parent: b3c9f3c56c41cbebe7804b48ba8e6e484509c2c0 > Refname:refs/heads/master > Author: Christian Gromm > AuthorDate: Tue Dec 22 10:53:06 2015 +0100 > Committer: Greg Kroah-Hartman > CommitDate: Sun Feb 7 17:34:58 2016 -0800 > > staging: most: remove stacked_mbo > > This patch makes use of kfifo_peek and kfifo_skip, which renders the > variable stacked_mbo useless. It is therefore removed. > > Signed-off-by: Christian Gromm > Signed-off-by: Greg Kroah-Hartman > --- > drivers/staging/most/aim-cdev/cdev.c | 16 +++- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/drivers/staging/most/aim-cdev/cdev.c > b/drivers/staging/most/aim-cdev/cdev.c > index d9c3f56..0ee2f08 100644 > --- a/drivers/staging/most/aim-cdev/cdev.c > +++ b/drivers/staging/most/aim-cdev/cdev.c > @@ -249,11 +246,7 @@ aim_read(struct file *filp, char __user *buf, size_t > count, loff_t *offset) > struct aim_channel *c = filp->private_data; > > mutex_lock(>io_mutex); > - if (c->stacked_mbo) { > - mbo = c->stacked_mbo; > - goto start_copy; > - } > - while ((!kfifo_out(>fifo, , 1)) && (c->dev)) { > + while (c->dev && !kfifo_peek(>fifo, )) { drivers/staging/most/aim-cdev/cdev.c:241: warning: ‘mbo’ may be used uninitialized in this function From looking at the code, it's not obvious to me if this is a false positive or not. Can it happen that mbo is not initialized fully, e.g. if less than sizeof(mbo) bytes have been read from the kfifo? Other callers initialize the pointer to NULL, and check the returned length. > mutex_unlock(>io_mutex); > if (filp->f_flags & O_NONBLOCK) > return -EAGAIN; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: most: warning: ‘mbo’ may be used uninitialized in this function
On Fri, Mar 18, 2016 at 01:41:19PM +0100, Geert Uytterhoeven wrote: > On Fri, Mar 18, 2016 at 6:42 AM, Linux Kernel Mailing List >wrote: > > Web: > > https://git.kernel.org/torvalds/c/f45b0fba43f415f69982df743dfa9b5d1b57785e > > Commit: f45b0fba43f415f69982df743dfa9b5d1b57785e > > Parent: b3c9f3c56c41cbebe7804b48ba8e6e484509c2c0 > > Refname:refs/heads/master > > Author: Christian Gromm > > AuthorDate: Tue Dec 22 10:53:06 2015 +0100 > > Committer: Greg Kroah-Hartman > > CommitDate: Sun Feb 7 17:34:58 2016 -0800 > > > > staging: most: remove stacked_mbo > > > > This patch makes use of kfifo_peek and kfifo_skip, which renders the > > variable stacked_mbo useless. It is therefore removed. > > > > Signed-off-by: Christian Gromm > > Signed-off-by: Greg Kroah-Hartman > > --- > > drivers/staging/most/aim-cdev/cdev.c | 16 +++- > > 1 file changed, 3 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/staging/most/aim-cdev/cdev.c > > b/drivers/staging/most/aim-cdev/cdev.c > > index d9c3f56..0ee2f08 100644 > > --- a/drivers/staging/most/aim-cdev/cdev.c > > +++ b/drivers/staging/most/aim-cdev/cdev.c > > > @@ -249,11 +246,7 @@ aim_read(struct file *filp, char __user *buf, size_t > > count, loff_t *offset) > > struct aim_channel *c = filp->private_data; > > > > mutex_lock(>io_mutex); > > - if (c->stacked_mbo) { > > - mbo = c->stacked_mbo; > > - goto start_copy; > > - } > > - while ((!kfifo_out(>fifo, , 1)) && (c->dev)) { > > + while (c->dev && !kfifo_peek(>fifo, )) { > > drivers/staging/most/aim-cdev/cdev.c:241: warning: ‘mbo’ may be used > uninitialized in this function > > From looking at the code, it's not obvious to me if this is a false > positive or not. > Can it happen that mbo is not initialized fully, e.g. if less than sizeof(mbo) > bytes have been read from the kfifo? > mbo is not touched by the kfifo_peek in the case where (c->dev == NULL), but since we protect the c->dev by the io_mutex it remains NULL until the check after the while loop where we quit. Looks like the analyzer suspects that c->dev may be changed to a valid pointer before the second check. So, it is false positive, but it is worth to initialize the mbo with the NULL to get rid of the warning. regards andy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel