Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1476521 > unrolled thread
| Started by | Colin King <colin.king@canonical.com> |
|---|---|
| First post | 2016-09-05 16:50 +0200 |
| Last post | 2016-09-07 21:20 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] iio: ensure ret is initialized to zero before entering do loop Colin King <colin.king@canonical.com> - 2016-09-05 16:50 +0200
Re: [PATCH] iio: ensure ret is initialized to zero before entering do loop Jonathan Cameron <jic23@kernel.org> - 2016-09-05 22:10 +0200
Re: [PATCH] iio: ensure ret is initialized to zero before entering do loop Brian Norris <briannorris@chromium.org> - 2016-09-06 19:20 +0200
Re: [PATCH] iio: ensure ret is initialized to zero before entering do loop Jonathan Cameron <jic23@kernel.org> - 2016-09-07 21:20 +0200
| From | Colin King <colin.king@canonical.com> |
|---|---|
| Date | 2016-09-05 16:50 +0200 |
| Subject | [PATCH] iio: ensure ret is initialized to zero before entering do loop |
| Message-ID | <se3nI-53R-31@gated-at.bofh.it> |
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")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
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);
remove_wait_queue(&rb->pollq, &wait);
return ret;
--
2.9.3
[toc] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-09-05 22:10 +0200 |
| Subject | Re: [PATCH] iio: ensure ret is initialized to zero before entering do loop |
| Message-ID | <se8nn-8w0-1@gated-at.bofh.it> |
| In reply to | #1476521 |
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")
> 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.
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);
> remove_wait_queue(&rb->pollq, &wait);
>
> return ret;
>
[toc] | [prev] | [next] | [standalone]
| From | Brian Norris <briannorris@chromium.org> |
|---|---|
| Date | 2016-09-06 19:20 +0200 |
| Subject | Re: [PATCH] iio: ensure ret is initialized to zero before entering do loop |
| Message-ID | <sescq-4T6-9@gated-at.bofh.it> |
| In reply to | #1476923 |
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;
> >
>
[toc] | [prev] | [next] | [standalone]
| From | Jonathan Cameron <jic23@kernel.org> |
|---|---|
| Date | 2016-09-07 21:20 +0200 |
| Subject | Re: [PATCH] iio: ensure ret is initialized to zero before entering do loop |
| Message-ID | <seQy6-3SW-35@gated-at.bofh.it> |
| In reply to | #1477678 |
On 06/09/16 18:10, Brian Norris wrote:
> 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>
Hi Brian,
Thanks for checking this out. Unfortunately I've already
applied it to fixes-togreg branch of iio.git which is strictly
non rebasing so we'll have to rely on the email trace rather
than git history to pick up on your tested by /reviewed by!
Made me more comfortable sending the pull request to Greg
though so thanks!
Jonathan
>
> 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;
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web