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

Groups > linux.kernel > #1565762 > unrolled thread

[PATCH 0/7] ALSA: Fine-tuning for several function implementations

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] ALSA: Fine-tuning for several function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-24 11:30 +0100
    [PATCH 7/7] dmasound_core: Adjust six function calls together with a  variable assignment SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-24 11:30 +0100
    [PATCH 2/7] ALSA: seq: Delete unnecessary checks in  snd_seq_ioctl_get_subscription() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-24 11:30 +0100
    [PATCH 1/7] ALSA: seq: Delete unnecessary checks in  snd_seq_ioctl_query_subs() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-24 11:30 +0100
    [PATCH 6/7] dmasound_core: Move two assignments for the variable  "ret" in state_open() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-24 11:30 +0100
      Re: [alsa-devel] [PATCH 6/7] dmasound_core: Move two assignments for  the variable "ret" in state_open() Clemens Ladisch <clemens@ladisch.de> - 2017-01-25 08:50 +0100
    [PATCH 5/7] ALSA: seq: Adjust 16 function calls together with a  variable assignment SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-24 11:30 +0100
    [PATCH 4/7] ALSA: seq: Delete unnecessary checks in  snd_seq_ioctl_subscribe_port() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-24 11:30 +0100

#1565762 — [PATCH 0/7] ALSA: Fine-tuning for several function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 24 Jan 2017 11:02:34 +0100

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (7):
  seq: Delete unnecessary checks in snd_seq_ioctl_query_subs()
  seq: Delete unnecessary checks in snd_seq_ioctl_get_subscription()
  seq: Delete unnecessary checks in snd_seq_ioctl_unsubscribe_port()
  seq: Delete unnecessary checks in snd_seq_ioctl_subscribe_port()
  seq: Adjust 16 function calls together with a variable assignment
  dmasound_core: Move two assignments for the variable "ret" in state_open()
  dmasound_core: Adjust six function calls together with a variable assignment

 sound/core/seq/seq_clientmgr.c     | 163 ++++++++++++++++++++-----------------
 sound/oss/dmasound/dmasound_core.c |  33 +++++---
 2 files changed, 109 insertions(+), 87 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1565763 — [PATCH 7/7] dmasound_core: Adjust six function calls together with a variable assignment

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 24 Jan 2017 10:34:47 +0100

The script "checkpatch.pl" pointed information out like the following.

ERROR: do not use assignment in if condition

Thus fix affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/oss/dmasound/dmasound_core.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/sound/oss/dmasound/dmasound_core.c b/sound/oss/dmasound/dmasound_core.c
index 2bdd1d619a7f..a20bf3b94329 100644
--- a/sound/oss/dmasound/dmasound_core.c
+++ b/sound/oss/dmasound/dmasound_core.c
@@ -576,7 +576,9 @@ static ssize_t sq_write(struct file *file, const char __user *src, size_t uLeft,
 	*/
 
 	if (write_sq.locked == 0) {
-		if ((uWritten = sq_setup(&write_sq)) < 0) return uWritten ;
+		uWritten = sq_setup(&write_sq);
+		if (uWritten < 0)
+			return uWritten;
 		uWritten = 0 ;
 	}
 
@@ -675,7 +677,8 @@ static unsigned int sq_poll(struct file *file, struct poll_table_struct *wait)
 	int retVal;
 	
 	if (write_sq.locked == 0) {
-		if ((retVal = sq_setup(&write_sq)) < 0)
+		retVal = sq_setup(&write_sq);
+		if (retVal < 0)
 			return retVal;
 		return 0;
 	}
@@ -736,7 +739,8 @@ static int sq_open2(struct sound_queue *sq, struct file *file, fmode_t mode,
 		   can't be changed at the moment - but _could_ be perhaps
 		   in the setfragments ioctl.
 		*/
-		if (( rc = sq_allocate_buffers(sq, numbufs, bufsize))) {
+		rc = sq_allocate_buffers(sq, numbufs, bufsize);
+		if (rc) {
 #if 0 /* blocking open() */
 			sq_wake_up(sq, file, mode);
 #else
@@ -1407,12 +1411,14 @@ int dmasound_init(void)
 	/* Set up sound queue, /dev/audio and /dev/dsp. */
 
 	/* Set default settings. */
-	if ((res = sq_init()) < 0)
-		return res ;
+	res = sq_init();
+	if (res < 0)
+		return res;
 
 	/* Set up /dev/sndstat. */
-	if ((res = state_init()) < 0)
-		return res ;
+	res = state_init();
+	if (res < 0)
+		return res;
 
 	/* Set up /dev/mixer. */
 	mixer_init();
@@ -1485,7 +1491,8 @@ static int dmasound_setup(char *str)
 			numWriteBufs = ints[1];
 		/* fall through */
 	case 1:
-		if ((size = ints[2]) < 256) /* check for small buffer specs */
+		size = ints[2];
+		if (size < 256) /* check for small buffer specs */
 			size <<= 10 ;
                 if (size < MIN_BUFSIZE || size > MAX_BUFSIZE)
                         printk("dmasound_setup: invalid write buffer size, using default = %d\n", writeBufSize);
-- 
2.11.0

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

#1565766 — [PATCH 2/7] ALSA: seq: Delete unnecessary checks in snd_seq_ioctl_get_subscription()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 24 Jan 2017 08:54:36 +0100

Two checks were repeated by the snd_seq_ioctl_get_subscription() function
despite of pointer determinations for the used variables at the beginning.

* Adjust jump targets according to the Linux coding style convention.

* Convert an assignment for the variable "result" to a direct initialisation.

* Delete extra variable initialisations and the repeated checks
  which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/core/seq/seq_clientmgr.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 594c341a193e..8e804ff784ed 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1891,16 +1891,15 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
 					  void *arg)
 {
 	struct snd_seq_port_subscribe *subs = arg;
-	int result;
-	struct snd_seq_client *sender = NULL;
-	struct snd_seq_client_port *sport = NULL;
+	int result = -EINVAL;
+	struct snd_seq_client *sender;
+	struct snd_seq_client_port *sport;
 	struct snd_seq_subscribers *p;
 
-	result = -EINVAL;
 	if ((sender = snd_seq_client_use_ptr(subs->sender.client)) == NULL)
-		goto __end;
+		goto exit;
 	if ((sport = snd_seq_port_use_ptr(sender, subs->sender.port)) == NULL)
-		goto __end;
+		goto unlock_client;
 	p = snd_seq_port_get_subscription(&sport->c_src, &subs->dest);
 	if (p) {
 		result = 0;
@@ -1908,12 +1907,10 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
 	} else
 		result = -ENOENT;
 
-      __end:
-      	if (sport)
-		snd_seq_port_unlock(sport);
-	if (sender)
-		snd_seq_client_unlock(sender);
-
+	snd_seq_port_unlock(sport);
+unlock_client:
+	snd_seq_client_unlock(sender);
+exit:
 	return result;
 }
 
-- 
2.11.0

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

#1565770 — [PATCH 1/7] ALSA: seq: Delete unnecessary checks in snd_seq_ioctl_query_subs()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 24 Jan 2017 08:00:34 +0100

Two checks were repeated by the snd_seq_ioctl_query_subs() function
despite of pointer determinations for the used variables at the beginning.

* Adjust jump targets according to the Linux coding style convention.

* Delete extra variable initialisations and the repeated checks
  which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/core/seq/seq_clientmgr.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 4c935202ce23..594c341a193e 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1925,16 +1925,16 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 {
 	struct snd_seq_query_subs *subs = arg;
 	int result = -ENXIO;
-	struct snd_seq_client *cptr = NULL;
-	struct snd_seq_client_port *port = NULL;
+	struct snd_seq_client *cptr;
+	struct snd_seq_client_port *port;
 	struct snd_seq_port_subs_info *group;
 	struct list_head *p;
 	int i;
 
 	if ((cptr = snd_seq_client_use_ptr(subs->root.client)) == NULL)
-		goto __end;
+		goto exit;
 	if ((port = snd_seq_port_use_ptr(cptr, subs->root.port)) == NULL)
-		goto __end;
+		goto unlock_client;
 
 	switch (subs->type) {
 	case SNDRV_SEQ_QUERY_SUBS_READ:
@@ -1944,7 +1944,7 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 		group = &port->c_dest;
 		break;
 	default:
-		goto __end;
+		goto unlock_port;
 	}
 
 	down_read(&group->list_mutex);
@@ -1970,13 +1970,11 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 		}
 	}
 	up_read(&group->list_mutex);
-
-      __end:
-   	if (port)
-		snd_seq_port_unlock(port);
-	if (cptr)
-		snd_seq_client_unlock(cptr);
-
+unlock_port:
+	snd_seq_port_unlock(port);
+unlock_client:
+	snd_seq_client_unlock(cptr);
+exit:
 	return result;
 }
 
-- 
2.11.0

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

#1565771 — [PATCH 6/7] dmasound_core: Move two assignments for the variable "ret" in state_open()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 24 Jan 2017 10:05:29 +0100

A local variable was set to an error code in two cases before a concrete
error situation was detected. Thus move the corresponding assignments into
if branches to indicate a software failure there.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/oss/dmasound/dmasound_core.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/sound/oss/dmasound/dmasound_core.c b/sound/oss/dmasound/dmasound_core.c
index f4ee85a4c42f..2bdd1d619a7f 100644
--- a/sound/oss/dmasound/dmasound_core.c
+++ b/sound/oss/dmasound/dmasound_core.c
@@ -1269,13 +1269,15 @@ static int state_open(struct inode *inode, struct file *file)
 	int ret;
 
 	mutex_lock(&dmasound_core_mutex);
-	ret = -EBUSY;
-	if (state.busy)
+	if (state.busy) {
+		ret = -EBUSY;
 		goto out;
+	}
 
-	ret = -ENODEV;
-	if (!try_module_get(dmasound.mach.owner))
+	if (!try_module_get(dmasound.mach.owner)) {
+		ret = -ENODEV;
 		goto out;
+	}
 
 	state.ptr = 0;
 	state.busy = 1;
-- 
2.11.0

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

#1566362 — Re: [alsa-devel] [PATCH 6/7] dmasound_core: Move two assignments for the variable "ret" in state_open()

SF Markus Elfring wrote:
> A local variable was set to an error code in two cases before a concrete
> error situation was detected.

And why would that be a problem?

http://yarchive.net/comp/linux/error_jumps.html

> -	ret = -EBUSY;
> -	if (state.busy)
> +	if (state.busy) {
> +		ret = -EBUSY;
>  		goto out;
> +	}


Regards,
Clemens

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

#1565772 — [PATCH 5/7] ALSA: seq: Adjust 16 function calls together with a variable assignment

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 24 Jan 2017 09:44:22 +0100

The script "checkpatch.pl" pointed information out like the following.

ERROR: do not use assignment in if condition

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/core/seq/seq_clientmgr.c | 52 ++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 17 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index b9518e7cb623..519968841ccc 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -400,7 +400,11 @@ static ssize_t snd_seq_read(struct file *file, char __user *buf, size_t count,
 	if (snd_BUG_ON(!client))
 		return -ENXIO;
 
-	if (!client->accept_input || (fifo = client->data.user.fifo) == NULL)
+	if (!client->accept_input)
+		return -ENXIO;
+
+	fifo = client->data.user.fifo;
+	if (!fifo)
 		return -ENXIO;
 
 	if (atomic_read(&fifo->overflow) > 0) {
@@ -419,9 +423,9 @@ static ssize_t snd_seq_read(struct file *file, char __user *buf, size_t count,
 		int nonblock;
 
 		nonblock = (file->f_flags & O_NONBLOCK) || result > 0;
-		if ((err = snd_seq_fifo_cell_out(fifo, &cell, nonblock)) < 0) {
+		err = snd_seq_fifo_cell_out(fifo, &cell, nonblock);
+		if (err < 0)
 			break;
-		}
 		if (snd_seq_ev_is_variable(&cell->event)) {
 			struct snd_seq_event tmpev;
 			tmpev = cell->event;
@@ -948,7 +952,8 @@ static int snd_seq_client_enqueue_event(struct snd_seq_client *client,
 		return err;
 
 	/* we got a cell. enqueue it. */
-	if ((err = snd_seq_enqueue_event(cell, atomic, hop)) < 0) {
+	err = snd_seq_enqueue_event(cell, atomic, hop);
+	if (err < 0) {
 		snd_seq_cell_free(cell);
 		return err;
 	}
@@ -1273,7 +1278,8 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void *arg)
 		return -EINVAL;
 	}
 	if (client->type == KERNEL_CLIENT) {
-		if ((callback = info->kernel) != NULL) {
+		callback = info->kernel;
+		if (callback) {
 			if (callback->owner)
 				port->owner = callback->owner;
 			port->private_data = callback->private_data;
@@ -1426,13 +1432,17 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
 	struct snd_seq_client *receiver, *sender;
 	struct snd_seq_client_port *sport, *dport;
 
-	if ((receiver = snd_seq_client_use_ptr(subs->dest.client)) == NULL)
+	receiver = snd_seq_client_use_ptr(subs->dest.client);
+	if (!receiver)
 		goto exit;
-	if ((sender = snd_seq_client_use_ptr(subs->sender.client)) == NULL)
+	sender = snd_seq_client_use_ptr(subs->sender.client);
+	if (!sender)
 		goto unlock_receiver;
-	if ((sport = snd_seq_port_use_ptr(sender, subs->sender.port)) == NULL)
+	sport = snd_seq_port_use_ptr(sender, subs->sender.port);
+	if (!sport)
 		goto unlock_sender;
-	if ((dport = snd_seq_port_use_ptr(receiver, subs->dest.port)) == NULL)
+	dport = snd_seq_port_use_ptr(receiver, subs->dest.port);
+	if (!dport)
 		goto unlock_sport;
 
 	result = check_subscription_permission(client, sport, dport, subs);
@@ -1468,13 +1478,17 @@ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,
 	struct snd_seq_client *receiver, *sender;
 	struct snd_seq_client_port *sport, *dport;
 
-	if ((receiver = snd_seq_client_use_ptr(subs->dest.client)) == NULL)
+	receiver = snd_seq_client_use_ptr(subs->dest.client);
+	if (!receiver)
 		goto exit;
-	if ((sender = snd_seq_client_use_ptr(subs->sender.client)) == NULL)
+	sender = snd_seq_client_use_ptr(subs->sender.client);
+	if (!sender)
 		goto unlock_receiver;
-	if ((sport = snd_seq_port_use_ptr(sender, subs->sender.port)) == NULL)
+	sport = snd_seq_port_use_ptr(sender, subs->sender.port);
+	if (!sport)
 		goto unlock_sender;
-	if ((dport = snd_seq_port_use_ptr(receiver, subs->dest.port)) == NULL)
+	dport = snd_seq_port_use_ptr(receiver, subs->dest.port);
+	if (!dport)
 		goto unlock_sport;
 
 	result = check_subscription_permission(client, sport, dport, subs);
@@ -1896,9 +1910,11 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
 	struct snd_seq_client_port *sport;
 	struct snd_seq_subscribers *p;
 
-	if ((sender = snd_seq_client_use_ptr(subs->sender.client)) == NULL)
+	sender = snd_seq_client_use_ptr(subs->sender.client);
+	if (!sender)
 		goto exit;
-	if ((sport = snd_seq_port_use_ptr(sender, subs->sender.port)) == NULL)
+	sport = snd_seq_port_use_ptr(sender, subs->sender.port);
+	if (!sport)
 		goto unlock_client;
 	p = snd_seq_port_get_subscription(&sport->c_src, &subs->dest);
 	if (p) {
@@ -1928,9 +1944,11 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
 	struct list_head *p;
 	int i;
 
-	if ((cptr = snd_seq_client_use_ptr(subs->root.client)) == NULL)
+	cptr = snd_seq_client_use_ptr(subs->root.client);
+	if (!cptr)
 		goto exit;
-	if ((port = snd_seq_port_use_ptr(cptr, subs->root.port)) == NULL)
+	port = snd_seq_port_use_ptr(cptr, subs->root.port);
+	if (!port)
 		goto unlock_client;
 
 	switch (subs->type) {
-- 
2.11.0

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

#1565773 — [PATCH 4/7] ALSA: seq: Delete unnecessary checks in snd_seq_ioctl_subscribe_port()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 24 Jan 2017 09:06:38 +0100

Four checks were repeated by the snd_seq_ioctl_subscribe_port() function
despite of pointer determinations for the used variables at the beginning.

* Adjust jump targets according to the Linux coding style convention.

* Delete extra variable initialisations and the repeated checks
  which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/core/seq/seq_clientmgr.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index ba26d622cb71..b9518e7cb623 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -1423,36 +1423,36 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
 {
 	struct snd_seq_port_subscribe *subs = arg;
 	int result = -EINVAL;
-	struct snd_seq_client *receiver = NULL, *sender = NULL;
-	struct snd_seq_client_port *sport = NULL, *dport = NULL;
+	struct snd_seq_client *receiver, *sender;
+	struct snd_seq_client_port *sport, *dport;
 
 	if ((receiver = snd_seq_client_use_ptr(subs->dest.client)) == NULL)
-		goto __end;
+		goto exit;
 	if ((sender = snd_seq_client_use_ptr(subs->sender.client)) == NULL)
-		goto __end;
+		goto unlock_receiver;
 	if ((sport = snd_seq_port_use_ptr(sender, subs->sender.port)) == NULL)
-		goto __end;
+		goto unlock_sender;
 	if ((dport = snd_seq_port_use_ptr(receiver, subs->dest.port)) == NULL)
-		goto __end;
+		goto unlock_sport;
 
 	result = check_subscription_permission(client, sport, dport, subs);
 	if (result < 0)
-		goto __end;
+		goto unlock_dport;
 
 	/* connect them */
 	result = snd_seq_port_connect(client, sender, sport, receiver, dport, subs);
 	if (! result) /* broadcast announce */
 		snd_seq_client_notify_subscription(SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0,
 						   subs, SNDRV_SEQ_EVENT_PORT_SUBSCRIBED);
-      __end:
-      	if (sport)
-		snd_seq_port_unlock(sport);
-	if (dport)
-		snd_seq_port_unlock(dport);
-	if (sender)
-		snd_seq_client_unlock(sender);
-	if (receiver)
-		snd_seq_client_unlock(receiver);
+unlock_dport:
+	snd_seq_port_unlock(dport);
+unlock_sport:
+	snd_seq_port_unlock(sport);
+unlock_sender:
+	snd_seq_client_unlock(sender);
+unlock_receiver:
+	snd_seq_client_unlock(receiver);
+exit:
 	return result;
 }
 
-- 
2.11.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web