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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2010-05-19 at 10:02 -0700, Greg KH wrote: 
> On Wed, May 19, 2010 at 09:50:54AM -0700, Joe Perches wrote:
> > 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.

Like long line lengths, braces placements and
checkpatch errors?

Perhaps this might be interesting:
$ git log --pretty=oneline drivers/staging/comedi/

It's all a piece.  Fixes are fixes.
Unnecessary code is unnecessary.

> > 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.

Of course a macro shouldn't have a return,
though a function would.

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux