On Tue, Jan 29, 2013 at 12:00:27PM +0000, Ian Abbott wrote:
> For comedi devices that support asynchronous commands on some of their
> subdevices, the comedi core creates extra device files for each of those
> subdevices of the form "/dev/comedi%i_subd%i". These use the same
> comedi device as the corresponding "/dev/comedi%i" but override the
> default "read" and "write" subdevice for the device. Currently it
> overrides both the read and write subdevice, but it only makes sense to
> override the "read" subdevice if the subdevice supports "read" commands,
> and to override the "write" subdevice if the subdevice supports "write"
> commands.
>
> In `comedi_alloc_subdevice_minor()`, only set `info->read_subdevice`
> non-NULL if the subdevice has the `SDF_CMD_READ` flag set, and only set
> `info->write_subdevice` non-NULL if the subdevice has the
> `SDF_CMD_WRITE` flag set. (`comedi_read_subdevice(info)` will use the
> device's default read subdevice if `info->read_subdevice` is NULL.
> `comedi_write_subdevice(info)` will use the device's default write
> subdevice if `info->write_subdevice` is NULL.
>
> Signed-off-by: Ian Abbott <[email protected]>
> ---
> drivers/staging/comedi/comedi_fops.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedi_fops.c
> b/drivers/staging/comedi/comedi_fops.c
> index b798e42..6bfaeef 100644
> --- a/drivers/staging/comedi/comedi_fops.c
> +++ b/drivers/staging/comedi/comedi_fops.c
> @@ -2358,8 +2358,10 @@ int comedi_alloc_subdevice_minor(struct comedi_device
> *dev,
> if (!info)
> return -ENOMEM;
> info->device = dev;
> - info->read_subdevice = s;
> - info->write_subdevice = s;
> + if ((s->subdev_flags & SDF_CMD_READ) != 0)
This patch is fine but why add != 0 to if statements?
This != 0 is only good style comparing against zero (the number, not
false or clear) and for cmp() functions.
if (strcmp(foo, bar) == 0) { ...
if (strcmp(foo, bar) < 0) { ...
if (strcmp(foo, bar) != 0) { ...
We can't compare strings directly but the C idiom is sort of like
saying:
if (foo == bar) { ...
if (foo < bar) { ...
if (foo != bar) { ...
If we left off the "!= 0" and just had "if (strcmp(foo, bar)) {..."
that's harder to translate mentally into "if (foo != bar) {".
But for normal conditions, adding additional != 0 to the end hurts
readability. If we're going to add one, why not add three?
if ((((s->subdev_flags & SDF_CMD_READ) != 0) != 0) != 0) {
It's just like in english we could append "is not false" to the end
of every if statement. "If you are going to the shop get some
eggs." becomes, "If you are going to the shop is not false get some
eggs."
regards,
dan carpenter
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
