Messages in this thread | | | Date | Tue, 6 Sep 2016 10:10:09 -0700 | From | Brian Norris <> | Subject | Re: [PATCH] iio: ensure ret is initialized to zero before entering do loop |
| |
Hi,
On Mon, Sep 05, 2016 at 09:03:26PM +0100, Jonathan Cameron wrote: > On 05/09/16 15:39, Colin King wrote: > > From: Colin Ian King <colin.king@canonical.com> > > > > A recent fix to iio_buffer_read_first_n_outer removed ret from being set by > > a return from wait_event_interruptible and also added a continue in a loop > > which causes the variable ret to not be set when it reaches the end of the > > loop. Fix this by initializing ret to zero. > > > > Also remove extraneous white space at the end of the loop. > > > > Fixes: fcf68f3c0bb2a5 ("fix sched WARNING "do not call blocking ops when !TASK_RUNNING")
Not that it really matters, but if the commit is still going to be amended at all, the subject was "iio: fix ...", not just "fix ...". Definitely not important though.
> > Signed-off-by: Colin Ian King <colin.king@canonical.com> > Good find. Strange that got through 0-day without a warning... > > Cc'd Brian as author of the fix this is fixing. > Brian can you sanity check this patch as well.
Indeed, looks fine, and works fine:
Tested-by: Brian Norris <briannorris@chromium.org> Reviewed-by: Brian Norris <briannorris@chromium.org>
Thanks for the fix Colin, and sorry for not noticing that error :(
> Applied to the fixes-togreg branch of iio.git and marked for stable. > Ah well, another one for the statistics on stable patches that introduce bugs while > fixing other bugs. > > Pretty unlikely this will be hit I think, but in theory you never know. > > Jonathan > > --- > > drivers/iio/industrialio-buffer.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c > > index 49bf9c5..158aaf4 100644 > > --- a/drivers/iio/industrialio-buffer.c > > +++ b/drivers/iio/industrialio-buffer.c > > @@ -110,7 +110,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf, > > DEFINE_WAIT_FUNC(wait, woken_wake_function); > > size_t datum_size; > > size_t to_wait; > > - int ret; > > + int ret = 0; > > > > if (!indio_dev->info) > > return -ENODEV; > > @@ -153,7 +153,7 @@ ssize_t iio_buffer_read_first_n_outer(struct file *filp, char __user *buf, > > ret = rb->access->read_first_n(rb, n, buf); > > if (ret == 0 && (filp->f_flags & O_NONBLOCK)) > > ret = -EAGAIN; > > - } while (ret == 0); > > + } while (ret == 0);
Personally, I avoided the temptation to fix the whitespace error in a bugfix patch. But this does scratch my itch :)
Brian
> > remove_wait_queue(&rb->pollq, &wait); > > > > return ret; > > >
| |