On Thu, Mar 17, 2011 at 11:29:08AM -0400, Andy Walls wrote:
> Jarod Wilson <ja...@redhat.com> wrote:
>
> >On Wed, Mar 16, 2011 at 08:07:22PM -0400, Andy Walls wrote:
> >> On Wed, 2011-03-16 at 16:24 -0400, Jarod Wilson wrote:
> >> > Signed-off-by: Jarod Wilson <ja...@redhat.com>
> >> > ---
> >> > drivers/staging/lirc/lirc_zilog.c | 4 ++++
> >> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/drivers/staging/lirc/lirc_zilog.c
> >b/drivers/staging/lirc/lirc_zilog.c
> >> > index 407d4b4..5ada643 100644
> >> > --- a/drivers/staging/lirc/lirc_zilog.c
> >> > +++ b/drivers/staging/lirc/lirc_zilog.c
> >> > @@ -950,6 +950,10 @@ static ssize_t read(struct file *filep, char
> >*outbuf, size_t n, loff_t *ppos)
> >> > ret = copy_to_user((void
> >> > *)outbuf+written, buf,
> >> > rbuf->chunk_size);
> >> > written += rbuf->chunk_size;
> >> > + } else {
> >> > + zilog_error("Buffer read failed!\n");
> >> > + ret = -EIO;
> >> > + break;
> >>
> >> No need to break, just let the non-0 ret value drop you out of the
> >while
> >> loop.
> >
> >Ah, indeed. I think I mindlessly copied what the tests just a few lines
> >above were doing without looking at the actual reason for them. I'll
> >remove that break from the patch here locally.
> >
> >--
> >Jarod Wilson
> >ja...@redhat.com
>
> You might also want to take a look at that test to ensure it doesn't break
> blocking read() behavior. (man 2 read). I'm swamped ATM and didn't look too
> hard.
>
> It seems odd that the lirc buffer object can have data ready (the first
> branch of the big if() in the while() loop), and yet the read of that lirc
> buffer object fails.
Generally, it shouldn't, but lirc_buffer_read uses kfifo underneath, and
in the pre-2.6.33 kfifo implementation, the retval from lirc_buffer_read
(as backported by way of media_build) is always 0, which is of course not
equal to chunk_size. So I think that in current kernels, this should never
trigger, and its partially just a note-to-self that this check will go
sideways when running on an older kernel, but not a bad check to have if
something really does go wrong.
--
Jarod Wilson
ja...@redhat.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
