On Wed, May 19, 2010 at 09:50:54AM -0700, Joe Perches wrote:
> On Wed, 2010-05-19 at 17:22 +0100, Ian Abbott wrote:
> > For write(), any data copied to the data buffer after the previously
> > set up streaming acquisition command has finished won't be used, but a
> > non-empty write() does not currently return 0 (or -EPIPE on error) after
> > the command has finished until the data buffer has been filled up.
> > Change this behavior to return 0 (or -EPIPE) any time after the command
> > has finished, without bothering to fill up the buffer with more useless
> > data.
> >
> > Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
> > ---
> > drivers/staging/comedi/comedi_fops.c | 23 +++++++++++++----------
> > 1 files changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
> > index 158a6b2..9a26428 100644
> > --- a/drivers/staging/comedi/comedi_fops.c
> > +++ b/drivers/staging/comedi/comedi_fops.c
> > @@ -1576,6 +1576,19 @@ static ssize_t comedi_write(struct file *file, const char __user *buf,
> > while (nbytes > 0 && !retval) {
> > set_current_state(TASK_INTERRUPTIBLE);
> >
> > + if (!(comedi_get_subdevice_runflags(s) & SRF_RUNNING)) {
> > + if (count == 0) {
> > + if (comedi_get_subdevice_runflags(s) &
> > + SRF_ERROR) {
> > + retval = -EPIPE;
> > + } else {
> > + retval = 0;
> > + }
> > + do_become_nonbusy(dev, s);
> > + }
> > + break;
> > + }
>
> Hi Ian.
>
> You tend to get many tab indentations.
In this case, it doesn't really matter. The comedi layer has much
bigger issues, please focus on the stuff that is important.
> As this is in a while (.. && !retval) the retval = 0 is unnecessary,
> so maybe something like this is better?
>
> if (!(comedi_get_subdevice_runflags(s) & SRF_RUNNING)) {
> if (count)
> break;
> if (comedi_get_subdevice_runflags(s) & SRF_ERROR)
> retval = -EPIPE;
> do_become_nonbusy(dev, s);
> break;
> }
>
> Maybe a macro/function like test_runflags might be useful
> to reduce the name/line length.
>
> #define test_runflags(s, flags) \
> return !!(comedi_get_subdevice_runflags(s) & flags);
_NEVER_ put a code path change in a #define. That makes it hell to
debug.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
