Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1435132 > unrolled thread

[PATCH v2 00/15] lirc_dev fixes and beautification

Started byAndi Shyti <andi.shyti@samsung.com>
First post2016-07-01 10:20 +0200
Last post2016-07-01 11:20 +0200
Articles 8 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 00/15] lirc_dev fixes and beautification Andi Shyti <andi.shyti@samsung.com> - 2016-07-01 10:20 +0200
    [PATCH v2 14/15] [media] lirc_dev: fix potential segfault Andi Shyti <andi.shyti@samsung.com> - 2016-07-01 10:30 +0200
    [PATCH v2 02/15] [media] lirc_dev: allow bufferless driver registration Andi Shyti <andi.shyti@samsung.com> - 2016-07-01 10:30 +0200
      Re: [PATCH v2 02/15] [media] lirc_dev: allow bufferless driver  registration Sean Young <sean@mess.org> - 2016-07-02 19:20 +0200
    [PATCH v2 12/15] [media] lirc_dev: fix error return value Andi Shyti <andi.shyti@samsung.com> - 2016-07-01 10:40 +0200
      Re: [PATCH v2 12/15] [media] lirc_dev: fix error return value Hans Verkuil <hverkuil@xs4all.nl> - 2016-07-04 13:50 +0200
    [PATCH v2 11/15] [media] lirc_dev: fix variable constant comparisons Andi Shyti <andi.shyti@samsung.com> - 2016-07-01 10:40 +0200
    [PATCH v2 09/15] [media] lirc_dev: merge three if statements in only  one Andi Shyti <andi.shyti@samsung.com> - 2016-07-01 11:20 +0200

#1435132 — [PATCH v2 00/15] lirc_dev fixes and beautification

FromAndi Shyti <andi.shyti@samsung.com>
Date2016-07-01 10:20 +0200
Subject[PATCH v2 00/15] lirc_dev fixes and beautification
Message-ID<rQ1Gp-4bf-7@gated-at.bofh.it>
Hi,

After applying Joe's suggestion, the next patches had some
conflicts, therefore I have to send all the 15 patches again.

This is a collection of fixes, added functionality, coding rework
and trivial coding style fixes.

The first patch is preparatory to the second, which allows the
user to create a lirc driver without receiver buffer, which is
obvious for transmitters. Besides, even though that buffer could
have been used also by transmitters, drivers might have the need
to handle it separately.

The rest of the patches is a series of coding style and code
rework, as I said, some of them are very trivial, but I sent them
anyway because I was on fire.

Patch 14 is a segfault fix, while the last patch adds the
possibility to send to ioctl the set frequency, get frequency and
set length command.

Changelog: V1->V2

 - As Joe recommended, in patch 4 I added the pr_fmt definition
   and removed all the hardcoded prefixes from the pr_*
   functions.

 - In Patch 15, after Sean's review, I removed the definitions of
   the GET/SET_FREQUENCY, I will use GET/SET_SEND_CARRIER
   instead, even though I find the name a bit confusing.

 - In patch 6 I did a better refactoring

Thanks,
Andi

Andi Shyti (15):
  [media] lirc_dev: place buffer allocation on separate function
  [media] lirc_dev: allow bufferless driver registration
  [media] lirc_dev: remove unnecessary debug prints
  [media] lirc_dev: replace printk with pr_* or dev_*
  [media] lirc_dev: simplify goto paths
  [media] lirc_dev: do not use goto to create loops
  [media] lirc_dev: simplify if statement in lirc_add_to_buf
  [media] lirc_dev: remove double if ... else statement
  [media] lirc_dev: merge three if statements in only one
  [media] lirc_dev: remove CONFIG_COMPAT precompiler check
  [media] lirc_dev: fix variable constant comparisons
  [media] lirc_dev: fix error return value
  [media] lirc_dev: extremely trivial comment style fix
  [media] lirc_dev: fix potential segfault
  [media] include: lirc: add LIRC_GET_LENGTH command

 drivers/media/rc/lirc_dev.c | 301 +++++++++++++++++++++-----------------------
 include/media/lirc_dev.h    |  12 ++
 include/uapi/linux/lirc.h   |   1 +
 3 files changed, 155 insertions(+), 159 deletions(-)

-- 
2.8.1

[toc] | [next] | [standalone]


#1435134 — [PATCH v2 14/15] [media] lirc_dev: fix potential segfault

FromAndi Shyti <andi.shyti@samsung.com>
Date2016-07-01 10:30 +0200
Subject[PATCH v2 14/15] [media] lirc_dev: fix potential segfault
Message-ID<rQ1ZL-4iE-1@gated-at.bofh.it>
In reply to#1435132
When opening or closing a lirc character device, the framework
provides to the user the possibility to keep track of opening or
closing of the device by calling two functions:

 - set_use_inc() when opening the device
 - set_use_dec() when closing the device

if those are not set by the lirc user, the system segfaults.
Check the pointer value before calling the above functions.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 4b3efcf..634779a 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -413,7 +413,10 @@ int lirc_unregister_driver(int minor)
 			ir->d.name, ir->d.minor);
 		wake_up_interruptible(&ir->buf->wait_poll);
 		mutex_lock(&ir->irctl_lock);
-		ir->d.set_use_dec(ir->d.data);
+
+		if (ir->d.set_use_dec)
+			ir->d.set_use_dec(ir->d.data);
+
 		module_put(cdev->owner);
 		mutex_unlock(&ir->irctl_lock);
 	} else {
@@ -471,7 +474,8 @@ int lirc_dev_fop_open(struct inode *inode, struct file *file)
 	cdev = ir->cdev;
 	if (try_module_get(cdev->owner)) {
 		ir->open++;
-		retval = ir->d.set_use_inc(ir->d.data);
+		if (ir->d.set_use_inc)
+			retval = ir->d.set_use_inc(ir->d.data);
 
 		if (retval) {
 			module_put(cdev->owner);
@@ -512,7 +516,8 @@ int lirc_dev_fop_close(struct inode *inode, struct file *file)
 
 	ir->open--;
 	if (ir->attached) {
-		ir->d.set_use_dec(ir->d.data);
+		if (ir->d.set_use_dec)
+			ir->d.set_use_dec(ir->d.data);
 		module_put(cdev->owner);
 	} else {
 		lirc_irctl_cleanup(ir);
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]


#1435139 — [PATCH v2 02/15] [media] lirc_dev: allow bufferless driver registration

FromAndi Shyti <andi.shyti@samsung.com>
Date2016-07-01 10:30 +0200
Subject[PATCH v2 02/15] [media] lirc_dev: allow bufferless driver registration
Message-ID<rQ1ZM-4iE-15@gated-at.bofh.it>
In reply to#1435132
Some drivers don't necessarily need to have a FIFO managed buffer
for their transfers. Drivers now should call
lirc_register_bufferless_driver in order to handle the buffer
themselves.

The function works exaclty like lirc_register_driver except of
the buffer allocation.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 44 ++++++++++++++++++++++++++++++++++----------
 include/media/lirc_dev.h    | 12 ++++++++++++
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 5716978..fa562a3 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -205,12 +205,14 @@ err_out:
 
 static int lirc_allocate_buffer(struct irctl *ir)
 {
-	int err;
+	int err = 0;
 	int bytes_in_key;
 	unsigned int chunk_size;
 	unsigned int buffer_size;
 	struct lirc_driver *d = &ir->d;
 
+	mutex_lock(&lirc_dev_lock);
+
 	bytes_in_key = BITS_TO_LONGS(d->code_length) +
 						(d->code_length % 8 ? 1 : 0);
 	buffer_size = d->buffer_size ? d->buffer_size : BUFLEN / bytes_in_key;
@@ -220,21 +222,26 @@ static int lirc_allocate_buffer(struct irctl *ir)
 		ir->buf = d->rbuf;
 	} else {
 		ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
-		if (!ir->buf)
-			return -ENOMEM;
+		if (!ir->buf) {
+			err = -ENOMEM;
+			goto out;
+		}
 
 		err = lirc_buffer_init(ir->buf, chunk_size, buffer_size);
 		if (err) {
 			kfree(ir->buf);
-			return err;
+			goto out;
 		}
 	}
 	ir->chunk_size = ir->buf->chunk_size;
 
-	return 0;
+out:
+	mutex_unlock(&lirc_dev_lock);
+
+	return err;
 }
 
-int lirc_register_driver(struct lirc_driver *d)
+static int lirc_allocate_driver(struct lirc_driver *d)
 {
 	struct irctl *ir;
 	int minor;
@@ -342,10 +349,6 @@ int lirc_register_driver(struct lirc_driver *d)
 	/* some safety check 8-) */
 	d->name[sizeof(d->name)-1] = '\0';
 
-	err = lirc_allocate_buffer(ir);
-	if (err)
-		goto out_lock;
-
 	if (d->features == 0)
 		d->features = LIRC_CAN_REC_LIRCCODE;
 
@@ -385,8 +388,29 @@ out_lock:
 out:
 	return err;
 }
+
+int lirc_register_driver(struct lirc_driver *d)
+{
+	int err, minor;
+
+	minor = lirc_allocate_driver(d);
+	if (minor < 0)
+		return minor;
+
+	err = lirc_allocate_buffer(irctls[minor]);
+	if (err)
+		lirc_unregister_driver(minor);
+
+	return err ? err : minor;
+}
 EXPORT_SYMBOL(lirc_register_driver);
 
+int lirc_register_bufferless_driver(struct lirc_driver *d)
+{
+	return lirc_allocate_driver(d);
+}
+EXPORT_SYMBOL(lirc_register_bufferless_driver);
+
 int lirc_unregister_driver(int minor)
 {
 	struct irctl *ir;
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 0ab59a5..8bed57a 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -214,6 +214,18 @@ struct lirc_driver {
  */
 extern int lirc_register_driver(struct lirc_driver *d);
 
+/* int lirc_register_bufferless_driver - allocates a lirc bufferless driver
+ * @d: reference to the lirc_driver to initialize
+ *
+ * The difference between lirc_register_driver and
+ * lirc_register_bufferless_driver is that the latter doesn't allocate any
+ * buffer, which means that the driver using the lirc_driver should take care of
+ * it by itself.
+ *
+ * returns 0 on success or a the negative errno number in case of failure.
+ */
+extern int lirc_register_bufferless_driver(struct lirc_driver *d);
+
 /* returns negative value on error or 0 if success
 */
 extern int lirc_unregister_driver(int minor);
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]


#1435844 — Re: [PATCH v2 02/15] [media] lirc_dev: allow bufferless driver registration

FromSean Young <sean@mess.org>
Date2016-07-02 19:20 +0200
SubjectRe: [PATCH v2 02/15] [media] lirc_dev: allow bufferless driver registration
Message-ID<rQwKd-6hM-1@gated-at.bofh.it>
In reply to#1435139
On Fri, Jul 01, 2016 at 05:01:25PM +0900, Andi Shyti wrote:
> Some drivers don't necessarily need to have a FIFO managed buffer
> for their transfers. Drivers now should call
> lirc_register_bufferless_driver in order to handle the buffer
> themselves.
> 
> The function works exaclty like lirc_register_driver except of
> the buffer allocation.

Indeed transmit-only devices don't need an input buffer, which is
just a waste of memory. However can't lirc_register_driver() figure
out from the features if the driver is capable of receiving, i.e.

int lirc_register_driver(struct lirc_driver *d)
{
	int err, minor;

	minor = lirc_allocate_driver(d);
	if (minor < 0)
		return minor;

	if (d->features & LIRC_CAN_REC_MODE2) {
		err = lirc_allocate_buffer(irctls[minor]);
		if (err)
			lirc_unregister_driver(minor);
	}

	return err ? err : minor;
}


Sean

> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/media/rc/lirc_dev.c | 44 ++++++++++++++++++++++++++++++++++----------
>  include/media/lirc_dev.h    | 12 ++++++++++++
>  2 files changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
> index 5716978..fa562a3 100644
> --- a/drivers/media/rc/lirc_dev.c
> +++ b/drivers/media/rc/lirc_dev.c
> @@ -205,12 +205,14 @@ err_out:
>  
>  static int lirc_allocate_buffer(struct irctl *ir)
>  {
> -	int err;
> +	int err = 0;
>  	int bytes_in_key;
>  	unsigned int chunk_size;
>  	unsigned int buffer_size;
>  	struct lirc_driver *d = &ir->d;
>  
> +	mutex_lock(&lirc_dev_lock);
> +
>  	bytes_in_key = BITS_TO_LONGS(d->code_length) +
>  						(d->code_length % 8 ? 1 : 0);
>  	buffer_size = d->buffer_size ? d->buffer_size : BUFLEN / bytes_in_key;
> @@ -220,21 +222,26 @@ static int lirc_allocate_buffer(struct irctl *ir)
>  		ir->buf = d->rbuf;
>  	} else {
>  		ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
> -		if (!ir->buf)
> -			return -ENOMEM;
> +		if (!ir->buf) {
> +			err = -ENOMEM;
> +			goto out;
> +		}
>  
>  		err = lirc_buffer_init(ir->buf, chunk_size, buffer_size);
>  		if (err) {
>  			kfree(ir->buf);
> -			return err;
> +			goto out;
>  		}
>  	}
>  	ir->chunk_size = ir->buf->chunk_size;
>  
> -	return 0;
> +out:
> +	mutex_unlock(&lirc_dev_lock);
> +
> +	return err;
>  }
>  
> -int lirc_register_driver(struct lirc_driver *d)
> +static int lirc_allocate_driver(struct lirc_driver *d)
>  {
>  	struct irctl *ir;
>  	int minor;
> @@ -342,10 +349,6 @@ int lirc_register_driver(struct lirc_driver *d)
>  	/* some safety check 8-) */
>  	d->name[sizeof(d->name)-1] = '\0';
>  
> -	err = lirc_allocate_buffer(ir);
> -	if (err)
> -		goto out_lock;
> -
>  	if (d->features == 0)
>  		d->features = LIRC_CAN_REC_LIRCCODE;
>  
> @@ -385,8 +388,29 @@ out_lock:
>  out:
>  	return err;
>  }
> +
> +int lirc_register_driver(struct lirc_driver *d)
> +{
> +	int err, minor;
> +
> +	minor = lirc_allocate_driver(d);
> +	if (minor < 0)
> +		return minor;
> +
> +	err = lirc_allocate_buffer(irctls[minor]);
> +	if (err)
> +		lirc_unregister_driver(minor);
> +
> +	return err ? err : minor;
> +}
>  EXPORT_SYMBOL(lirc_register_driver);
>  
> +int lirc_register_bufferless_driver(struct lirc_driver *d)
> +{
> +	return lirc_allocate_driver(d);
> +}
> +EXPORT_SYMBOL(lirc_register_bufferless_driver);
> +
>  int lirc_unregister_driver(int minor)
>  {
>  	struct irctl *ir;
> diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
> index 0ab59a5..8bed57a 100644
> --- a/include/media/lirc_dev.h
> +++ b/include/media/lirc_dev.h
> @@ -214,6 +214,18 @@ struct lirc_driver {
>   */
>  extern int lirc_register_driver(struct lirc_driver *d);
>  
> +/* int lirc_register_bufferless_driver - allocates a lirc bufferless driver
> + * @d: reference to the lirc_driver to initialize
> + *
> + * The difference between lirc_register_driver and
> + * lirc_register_bufferless_driver is that the latter doesn't allocate any
> + * buffer, which means that the driver using the lirc_driver should take care of
> + * it by itself.
> + *
> + * returns 0 on success or a the negative errno number in case of failure.
> + */
> +extern int lirc_register_bufferless_driver(struct lirc_driver *d);
> +
>  /* returns negative value on error or 0 if success
>  */
>  extern int lirc_unregister_driver(int minor);
> -- 
> 2.8.1

[toc] | [prev] | [next] | [standalone]


#1435152 — [PATCH v2 12/15] [media] lirc_dev: fix error return value

FromAndi Shyti <andi.shyti@samsung.com>
Date2016-07-01 10:40 +0200
Subject[PATCH v2 12/15] [media] lirc_dev: fix error return value
Message-ID<rQ29r-4m1-17@gated-at.bofh.it>
In reply to#1435132
If ioctl is called, it cannot be a case of invalid system call
number (ENOSYS), that is an operation not permitted (EPERM).
Replace ENOSYS with EPERM.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 689e369..99d1f98 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -587,7 +587,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		break;
 	case LIRC_GET_REC_MODE:
 		if (!(ir->d.features & LIRC_CAN_REC_MASK)) {
-			result = -ENOSYS;
+			result = -EPERM;
 			break;
 		}
 
@@ -597,7 +597,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		break;
 	case LIRC_SET_REC_MODE:
 		if (!(ir->d.features & LIRC_CAN_REC_MASK)) {
-			result = -ENOSYS;
+			result = -EPERM;
 			break;
 		}
 
@@ -615,7 +615,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case LIRC_GET_MIN_TIMEOUT:
 		if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
 		    ir->d.min_timeout == 0) {
-			result = -ENOSYS;
+			result = -EPERM;
 			break;
 		}
 
@@ -624,7 +624,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	case LIRC_GET_MAX_TIMEOUT:
 		if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
 		    ir->d.max_timeout == 0) {
-			result = -ENOSYS;
+			result = -EPERM;
 			break;
 		}
 
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]


#1436428 — Re: [PATCH v2 12/15] [media] lirc_dev: fix error return value

FromHans Verkuil <hverkuil@xs4all.nl>
Date2016-07-04 13:50 +0200
SubjectRe: [PATCH v2 12/15] [media] lirc_dev: fix error return value
Message-ID<rRaxX-55H-1@gated-at.bofh.it>
In reply to#1435152
On 07/01/2016 10:01 AM, Andi Shyti wrote:
> If ioctl is called, it cannot be a case of invalid system call
> number (ENOSYS), that is an operation not permitted (EPERM).
> Replace ENOSYS with EPERM.

I'd say it is ENOTTY, i.e. this hardware does not support this ioctl.

Regards,

	Hans

> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/media/rc/lirc_dev.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
> index 689e369..99d1f98 100644
> --- a/drivers/media/rc/lirc_dev.c
> +++ b/drivers/media/rc/lirc_dev.c
> @@ -587,7 +587,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  		break;
>  	case LIRC_GET_REC_MODE:
>  		if (!(ir->d.features & LIRC_CAN_REC_MASK)) {
> -			result = -ENOSYS;
> +			result = -EPERM;
>  			break;
>  		}
>  
> @@ -597,7 +597,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  		break;
>  	case LIRC_SET_REC_MODE:
>  		if (!(ir->d.features & LIRC_CAN_REC_MASK)) {
> -			result = -ENOSYS;
> +			result = -EPERM;
>  			break;
>  		}
>  
> @@ -615,7 +615,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  	case LIRC_GET_MIN_TIMEOUT:
>  		if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
>  		    ir->d.min_timeout == 0) {
> -			result = -ENOSYS;
> +			result = -EPERM;
>  			break;
>  		}
>  
> @@ -624,7 +624,7 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  	case LIRC_GET_MAX_TIMEOUT:
>  		if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
>  		    ir->d.max_timeout == 0) {
> -			result = -ENOSYS;
> +			result = -EPERM;
>  			break;
>  		}
>  
> 

[toc] | [prev] | [next] | [standalone]


#1435154 — [PATCH v2 11/15] [media] lirc_dev: fix variable constant comparisons

FromAndi Shyti <andi.shyti@samsung.com>
Date2016-07-01 10:40 +0200
Subject[PATCH v2 11/15] [media] lirc_dev: fix variable constant comparisons
Message-ID<rQ29s-4m1-29@gated-at.bofh.it>
In reply to#1435132
When comparing a variable with a constant, the comparison should
start from the variable and not from the constant. It's also
written in the human DNA.

Swap the terms of comparisons whenever the constant comes first
and fix the following checkpatch warning:

  WARNING: Comparisons should place the constant on the right side of the test

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 16cca46..689e369 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -246,13 +246,13 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 		return -EINVAL;
 	}
 
-	if (MAX_IRCTL_DEVICES <= d->minor) {
+	if (d->minor >= MAX_IRCTL_DEVICES) {
 		dev_err(d->dev, "minor must be between 0 and %d!\n",
 						MAX_IRCTL_DEVICES - 1);
 		return -EBADRQC;
 	}
 
-	if (1 > d->code_length || (BUFLEN * 8) < d->code_length) {
+	if (d->code_length < 1 || d->code_length > (BUFLEN * 8)) {
 		dev_err(d->dev, "code length must be less than %d bits\n",
 								BUFLEN * 8);
 		return -EBADRQC;
@@ -283,7 +283,7 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 		for (minor = 0; minor < MAX_IRCTL_DEVICES; minor++)
 			if (!irctls[minor])
 				break;
-		if (MAX_IRCTL_DEVICES == minor) {
+		if (minor == MAX_IRCTL_DEVICES) {
 			dev_err(d->dev, "no free slots for drivers!\n");
 			err = -ENOMEM;
 			goto out_lock;
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]


#1435181 — [PATCH v2 09/15] [media] lirc_dev: merge three if statements in only one

FromAndi Shyti <andi.shyti@samsung.com>
Date2016-07-01 11:20 +0200
Subject[PATCH v2 09/15] [media] lirc_dev: merge three if statements in only one
Message-ID<rQ2M9-4Op-13@gated-at.bofh.it>
In reply to#1435132
The three if statements check the same thing, merge them in only
one statement.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/media/rc/lirc_dev.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 2643336..d98a9f1 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -270,15 +270,10 @@ static int lirc_allocate_driver(struct lirc_driver *d)
 			dev_err(d->dev, "add_to_buf not set\n");
 			return -EBADRQC;
 		}
-	} else if (!(d->fops && d->fops->read) && !d->rbuf) {
-		dev_err(d->dev, "fops->read and rbuf are NULL!\n");
+	} else if (!d->rbuf && !(d->fops && d->fops->read &&
+				d->fops->poll && d->fops->unlocked_ioctl)) {
+		dev_err(d->dev, "undefined read, poll, ioctl\n");
 		return -EBADRQC;
-	} else if (!d->rbuf) {
-		if (!(d->fops && d->fops->read && d->fops->poll &&
-		      d->fops->unlocked_ioctl)) {
-			dev_err(d->dev, "undefined read, poll, ioctl\n");
-			return -EBADRQC;
-		}
 	}
 
 	mutex_lock(&lirc_dev_lock);
-- 
2.8.1

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web