[PATCH] Staging: comedi: don't write to buffer if command finished

[Greg KH]

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 at mev.co.uk>
> > ---
> >  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