[PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations

linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations
@ 2016-12-31 21:54 SF Markus Elfring
  2016-12-31 21:58 ` [PATCH 1/6] staging: vchiq_arm: Use kmalloc_array() in dump_phys_mem() SF Markus Elfring
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: SF Markus Elfring @ 2016-12-31 21:54 UTC (permalink / raw)
  To: linux-arm-kernel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Dec 2016 22:42:34 +0100

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

Markus Elfring (6):
  Use kmalloc_array() in dump_phys_mem()
  Adjust 13 checks for null pointers
  One check less in dump_phys_mem() after error detection
  Delete an error message for a failed memory allocation in dump_phys_mem()
  Combine substrings for 24 messages
  Delete an unnecessary return statement in two functions

 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 212 +++++++++++----------
 1 file changed, 109 insertions(+), 103 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/6] staging: vchiq_arm: Use kmalloc_array() in dump_phys_mem()
  2016-12-31 21:54 [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations SF Markus Elfring
@ 2016-12-31 21:58 ` SF Markus Elfring
  2016-12-31 22:00 ` [PATCH 2/6] staging: vchiq_arm: Adjust 13 checks for null pointers SF Markus Elfring
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: SF Markus Elfring @ 2016-12-31 21:58 UTC (permalink / raw)
  To: linux-arm-kernel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Dec 2016 17:50:25 +0100

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 610e0d6d93bb..4af77d790ae0 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1558,8 +1558,7 @@ dump_phys_mem(void *virt_addr, uint32_t num_bytes)
 	end_offset = (int)(long)end_virt_addr & (PAGE_SIZE - 1);
 
 	num_pages = (offset + num_bytes + PAGE_SIZE - 1) / PAGE_SIZE;
-
-	pages = kmalloc(sizeof(struct page *) * num_pages, GFP_KERNEL);
+	pages = kmalloc_array(num_pages, sizeof(*pages), GFP_KERNEL);
 	if (pages == NULL) {
 		vchiq_log_error(vchiq_arm_log_level,
 			"Unable to allocation memory for %d pages\n",
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/6] staging: vchiq_arm: Adjust 13 checks for null pointers
  2016-12-31 21:54 [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations SF Markus Elfring
  2016-12-31 21:58 ` [PATCH 1/6] staging: vchiq_arm: Use kmalloc_array() in dump_phys_mem() SF Markus Elfring
@ 2016-12-31 22:00 ` SF Markus Elfring
  2016-12-31 22:01 ` [PATCH 3/6] staging: vchiq_arm: One check less in dump_phys_mem() after error detection SF Markus Elfring
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: SF Markus Elfring @ 2016-12-31 22:00 UTC (permalink / raw)
  To: linux-arm-kernel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Dec 2016 21:23:24 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

Comparison to NULL could be written ?

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 34 ++++++++++------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 4af77d790ae0..2a260034189d 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -533,7 +533,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		/* Remove all services */
 		i = 0;
 		while ((service = next_service_by_instance(instance->state,
-			instance, &i)) != NULL) {
+							   instance, &i))) {
 			status = vchiq_remove_service(service->handle);
 			unlock_service(service);
 			if (status != VCHIQ_SUCCESS)
@@ -614,7 +614,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 				&args.params, srvstate,
 				instance, user_service_free);
 
-		if (service != NULL) {
+		if (service) {
 			user_service->service = service;
 			user_service->userdata = userdata;
 			user_service->instance = instance;
@@ -661,7 +661,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
 
 		service = find_service_for_instance(instance, handle);
-		if (service != NULL) {
+		if (service) {
 			USER_SERVICE_T *user_service =
 				(USER_SERVICE_T *)service->base.userdata;
 			/* close_pending is false on first entry, and when the
@@ -687,7 +687,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
 
 		service = find_service_for_instance(instance, handle);
-		if (service != NULL) {
+		if (service) {
 			USER_SERVICE_T *user_service =
 				(USER_SERVICE_T *)service->base.userdata;
 			/* close_pending is false on first entry, and when the
@@ -714,7 +714,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
 
 		service = find_service_for_instance(instance, handle);
-		if (service != NULL) {
+		if (service) {
 			status = (cmd == VCHIQ_IOC_USE_SERVICE)	?
 				vchiq_use_service_internal(service) :
 				vchiq_release_service_internal(service);
@@ -747,7 +747,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		service = find_service_for_instance(instance, args.handle);
 
-		if ((service != NULL) && (args.count <= MAX_ELEMENTS)) {
+		if (service && (args.count <= MAX_ELEMENTS)) {
 			/* Copy elements into kernel space */
 			VCHIQ_ELEMENT_T elements[MAX_ELEMENTS];
 			if (copy_from_user(elements, args.elements,
@@ -1063,11 +1063,11 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		spin_unlock(&msg_queue_spinlock);
 
 		up(&user_service->remove_event);
-		if (header == NULL)
+		if (!header)
 			ret = -ENOTCONN;
 		else if (header->size <= args.bufsize) {
 			/* Copy to user space if msgbuf is not NULL */
-			if ((args.buf == NULL) ||
+			if (!args.buf ||
 				(copy_to_user((void __user *)args.buf,
 				header->data,
 				header->size) == 0)) {
@@ -1161,7 +1161,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		VCHIQ_SERVICE_HANDLE_T handle = (VCHIQ_SERVICE_HANDLE_T)arg;
 
 		service = find_closed_service_for_instance(instance, handle);
-		if (service != NULL) {
+		if (service) {
 			USER_SERVICE_T *user_service =
 				(USER_SERVICE_T *)service->base.userdata;
 			close_delivered(user_service);
@@ -1559,7 +1559,7 @@ dump_phys_mem(void *virt_addr, uint32_t num_bytes)
 
 	num_pages = (offset + num_bytes + PAGE_SIZE - 1) / PAGE_SIZE;
 	pages = kmalloc_array(num_pages, sizeof(*pages), GFP_KERNEL);
-	if (pages == NULL) {
+	if (!pages) {
 		vchiq_log_error(vchiq_arm_log_level,
 			"Unable to allocation memory for %d pages\n",
 			num_pages);
@@ -1590,8 +1590,7 @@ dump_phys_mem(void *virt_addr, uint32_t num_bytes)
 		page_idx = offset / PAGE_SIZE;
 
 		if (page_idx != prev_idx) {
-
-			if (page != NULL)
+			if (page)
 				kunmap(page);
 			page = pages[page_idx];
 			kmapped_virt_ptr = kmap(page);
@@ -1609,7 +1608,7 @@ dump_phys_mem(void *virt_addr, uint32_t num_bytes)
 	}
 
 out:
-	if (page != NULL)
+	if (page)
 		kunmap(page);
 
 	for (page_idx = 0; page_idx < num_pages; page_idx++)
@@ -1644,14 +1643,13 @@ vchiq_read(struct file *file, char __user *buf,
 VCHIQ_STATE_T *
 vchiq_get_state(void)
 {
-
-	if (g_state.remote == NULL)
+	if (!g_state.remote)
 		printk(KERN_ERR "%s: g_state.remote == NULL\n", __func__);
 	else if (g_state.remote->initialised != 1)
 		printk(KERN_NOTICE "%s: g_state.remote->initialised != 1 (%d)\n",
 			__func__, g_state.remote->initialised);
 
-	return ((g_state.remote != NULL) &&
+	return (g_state.remote &&
 		(g_state.remote->initialised == 1)) ? &g_state : NULL;
 }
 
@@ -2655,7 +2653,7 @@ vchiq_instance_get_use_count(VCHIQ_INSTANCE_T instance)
 	int use_count = 0, i;
 	i = 0;
 	while ((service = next_service_by_instance(instance->state,
-		instance, &i)) != NULL) {
+						   instance, &i))) {
 		use_count += service->service_use_count;
 		unlock_service(service);
 	}
@@ -2681,7 +2679,7 @@ vchiq_instance_set_trace(VCHIQ_INSTANCE_T instance, int trace)
 	int i;
 	i = 0;
 	while ((service = next_service_by_instance(instance->state,
-		instance, &i)) != NULL) {
+						   instance, &i))) {
 		service->trace = trace;
 		unlock_service(service);
 	}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/6] staging: vchiq_arm: One check less in dump_phys_mem() after error detection
  2016-12-31 21:54 [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations SF Markus Elfring
  2016-12-31 21:58 ` [PATCH 1/6] staging: vchiq_arm: Use kmalloc_array() in dump_phys_mem() SF Markus Elfring
  2016-12-31 22:00 ` [PATCH 2/6] staging: vchiq_arm: Adjust 13 checks for null pointers SF Markus Elfring
@ 2016-12-31 22:01 ` SF Markus Elfring
  2016-12-31 22:03 ` [PATCH 4/6] staging: vchiq_arm: Delete an error message for a failed memory allocation in dump_phys_mem() SF Markus Elfring
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: SF Markus Elfring @ 2016-12-31 22:01 UTC (permalink / raw)
  To: linux-arm-kernel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Dec 2016 21:26:09 +0100

Adjust a jump target according to the Linux coding style convention
so that a redundant check for a null pointer can be avoided
in this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 2a260034189d..a316cf9ac626 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1581,7 +1581,7 @@ dump_phys_mem(void *virt_addr, uint32_t num_bytes)
 	if (rc < 0) {
 		vchiq_log_error(vchiq_arm_log_level,
 				"Failed to get user pages: %d\n", rc);
-		goto out;
+		goto put_pages;
 	}
 
 	while (offset < end_offset) {
@@ -1607,10 +1607,9 @@ dump_phys_mem(void *virt_addr, uint32_t num_bytes)
 		offset += 16;
 	}
 
-out:
 	if (page)
 		kunmap(page);
-
+put_pages:
 	for (page_idx = 0; page_idx < num_pages; page_idx++)
 		put_page(pages[page_idx]);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/6] staging: vchiq_arm: Delete an error message for a failed memory allocation in dump_phys_mem()
  2016-12-31 21:54 [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations SF Markus Elfring
                   ` (2 preceding siblings ...)
  2016-12-31 22:01 ` [PATCH 3/6] staging: vchiq_arm: One check less in dump_phys_mem() after error detection SF Markus Elfring
@ 2016-12-31 22:03 ` SF Markus Elfring
  2016-12-31 22:04 ` [PATCH 5/6] staging: vchiq_arm: Combine substrings for 24 messages SF Markus Elfring
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: SF Markus Elfring @ 2016-12-31 22:03 UTC (permalink / raw)
  To: linux-arm-kernel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Dec 2016 21:30:31 +0100

Omit an extra message for a memory allocation failure in this function.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index a316cf9ac626..3b7a0c87954d 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1559,12 +1559,8 @@ dump_phys_mem(void *virt_addr, uint32_t num_bytes)
 
 	num_pages = (offset + num_bytes + PAGE_SIZE - 1) / PAGE_SIZE;
 	pages = kmalloc_array(num_pages, sizeof(*pages), GFP_KERNEL);
-	if (!pages) {
-		vchiq_log_error(vchiq_arm_log_level,
-			"Unable to allocation memory for %d pages\n",
-			num_pages);
+	if (!pages)
 		return;
-	}
 
 	down_read(&current->mm->mmap_sem);
 	rc = get_user_pages(
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/6] staging: vchiq_arm: Combine substrings for 24 messages
  2016-12-31 21:54 [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations SF Markus Elfring
                   ` (3 preceding siblings ...)
  2016-12-31 22:03 ` [PATCH 4/6] staging: vchiq_arm: Delete an error message for a failed memory allocation in dump_phys_mem() SF Markus Elfring
@ 2016-12-31 22:04 ` SF Markus Elfring
  2016-12-31 22:06 ` [PATCH 6/6] staging: vchiq_arm: Delete an unnecessary return statement in two functions SF Markus Elfring
  2017-01-03 17:46 ` [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations Eric Anholt
  6 siblings, 0 replies; 9+ messages in thread
From: SF Markus Elfring @ 2016-12-31 22:04 UTC (permalink / raw)
  To: linux-arm-kernel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Dec 2016 22:00:28 +0100

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

WARNING: quoted string split across lines

* Thus fix the affected source code places.

* Improve indentation for passed parameters.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 164 +++++++++++----------
 1 file changed, 90 insertions(+), 74 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 3b7a0c87954d..05a00914dba0 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -292,12 +292,14 @@ service_callback(VCHIQ_REASON_T reason, VCHIQ_HEADER_T *header,
 		return VCHIQ_SUCCESS;
 
 	vchiq_log_trace(vchiq_arm_log_level,
-		"service_callback - service %lx(%d,%p), reason %d, header %lx, "
-		"instance %lx, bulk_userdata %lx",
-		(unsigned long)user_service,
-		service->localport, user_service->userdata,
-		reason, (unsigned long)header,
-		(unsigned long)instance, (unsigned long)bulk_userdata);
+			"service_callback - service %lx(%d,%p), reason %d, header %lx, instance %lx, bulk_userdata %lx",
+			(unsigned long)user_service,
+			service->localport,
+			user_service->userdata,
+			reason,
+			(unsigned long)header,
+			(unsigned long)instance,
+			(unsigned long)bulk_userdata);
 
 	if (header && user_service->is_vchi) {
 		spin_lock(&msg_queue_spinlock);
@@ -557,9 +559,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		rc = mutex_lock_killable(&instance->state->mutex);
 		if (rc != 0) {
 			vchiq_log_error(vchiq_arm_log_level,
-				"vchiq: connect: could not lock mutex for "
-				"state %d: %d",
-				instance->state->id, rc);
+					"vchiq: connect: could not lock mutex for state %d: %d",
+					instance->state->id, rc);
 			ret = -EINTR;
 			break;
 		}
@@ -720,16 +721,14 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 				vchiq_release_service_internal(service);
 			if (status != VCHIQ_SUCCESS) {
 				vchiq_log_error(vchiq_susp_log_level,
-					"%s: cmd %s returned error %d for "
-					"service %c%c%c%c:%03d",
-					__func__,
-					(cmd == VCHIQ_IOC_USE_SERVICE) ?
-						"VCHIQ_IOC_USE_SERVICE" :
-						"VCHIQ_IOC_RELEASE_SERVICE",
-					status,
-					VCHIQ_FOURCC_AS_4CHARS(
-						service->base.fourcc),
-					service->client_id);
+						"%s: cmd %s returned error %d for service %c%c%c%c:%03d",
+						__func__,
+						(cmd == VCHIQ_IOC_USE_SERVICE)
+						? "VCHIQ_IOC_USE_SERVICE"
+						: "VCHIQ_IOC_RELEASE_SERVICE",
+						status,
+						VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc),
+						service->client_id);
 				ret = -EINVAL;
 			}
 		} else
@@ -930,8 +929,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 							"header %pK: msgbufsize %x < msglen %x",
 							header, args.msgbufsize,
 							msglen);
-						WARN(1, "invalid message "
-							"size\n");
+						WARN(1,
+						     "invalid message size\n");
 						if (ret == 0)
 							ret = -EMSGSIZE;
 						break;
@@ -1982,19 +1981,22 @@ block_resume(VCHIQ_ARM_STATE_T *arm_state)
 	if (arm_state->blocked_count) {
 		reinit_completion(&arm_state->blocked_blocker);
 		write_unlock_bh(&arm_state->susp_res_lock);
-		vchiq_log_info(vchiq_susp_log_level, "%s wait for previously "
-			"blocked clients", __func__);
+		vchiq_log_info(vchiq_susp_log_level,
+			       "%s wait for previously blocked clients",
+			       __func__);
 		if (wait_for_completion_interruptible_timeout(
 				&arm_state->blocked_blocker, timeout_val)
 					<= 0) {
-			vchiq_log_error(vchiq_susp_log_level, "%s wait for "
-				"previously blocked clients failed" , __func__);
+			vchiq_log_error(vchiq_susp_log_level,
+					"%s wait for previously blocked clients failed",
+					__func__);
 			status = VCHIQ_ERROR;
 			write_lock_bh(&arm_state->susp_res_lock);
 			goto out;
 		}
-		vchiq_log_info(vchiq_susp_log_level, "%s previously blocked "
-			"clients resumed", __func__);
+		vchiq_log_info(vchiq_susp_log_level,
+			       "%s previously blocked clients resumed",
+			       __func__);
 		write_lock_bh(&arm_state->susp_res_lock);
 	}
 
@@ -2003,8 +2005,9 @@ block_resume(VCHIQ_ARM_STATE_T *arm_state)
 			arm_state->vc_resume_state > VC_RESUME_IDLE) {
 		if (resume_count > 1) {
 			status = VCHIQ_ERROR;
-			vchiq_log_error(vchiq_susp_log_level, "%s waited too "
-				"many times for resume" , __func__);
+			vchiq_log_error(vchiq_susp_log_level,
+					"%s waited too many times for resume",
+					__func__);
 			goto out;
 		}
 		write_unlock_bh(&arm_state->susp_res_lock);
@@ -2013,10 +2016,11 @@ block_resume(VCHIQ_ARM_STATE_T *arm_state)
 		if (wait_for_completion_interruptible_timeout(
 				&arm_state->vc_resume_complete, timeout_val)
 					<= 0) {
-			vchiq_log_error(vchiq_susp_log_level, "%s wait for "
-				"resume failed (%s)", __func__,
-				resume_state_names[arm_state->vc_resume_state +
-							VC_RESUME_NUM_OFFSET]);
+			vchiq_log_error(vchiq_susp_log_level,
+					"%s wait for resume failed (%s)",
+					__func__,
+					resume_state_names[arm_state->vc_resume_state
+							   + VC_RESUME_NUM_OFFSET]);
 			status = VCHIQ_ERROR;
 			write_lock_bh(&arm_state->susp_res_lock);
 			goto out;
@@ -2056,12 +2060,14 @@ vchiq_arm_vcsuspend(VCHIQ_STATE_T *state)
 
 	switch (arm_state->vc_suspend_state) {
 	case VC_SUSPEND_REQUESTED:
-		vchiq_log_info(vchiq_susp_log_level, "%s: suspend already "
-			"requested", __func__);
+		vchiq_log_info(vchiq_susp_log_level,
+			       "%s: suspend already requested",
+			       __func__);
 		break;
 	case VC_SUSPEND_IN_PROGRESS:
-		vchiq_log_info(vchiq_susp_log_level, "%s: suspend already in "
-			"progress", __func__);
+		vchiq_log_info(vchiq_susp_log_level,
+			       "%s: suspend already in progress",
+			       __func__);
 		break;
 
 	default:
@@ -2136,13 +2142,14 @@ output_timeout_error(VCHIQ_STATE_T *state)
 		VCHIQ_SERVICE_T *service_ptr = state->services[i];
 		if (service_ptr && service_ptr->service_use_count &&
 			(service_ptr->srvstate != VCHIQ_SRVSTATE_FREE)) {
-			snprintf(err, sizeof(err), " %c%c%c%c(%d) service has "
-				"use count %d%s", VCHIQ_FOURCC_AS_4CHARS(
-					service_ptr->base.fourcc),
+			snprintf(err, sizeof(err),
+				 " %c%c%c%c(%d) service has use count %d%s",
+				 VCHIQ_FOURCC_AS_4CHARS(service_ptr->base.fourcc),
 				 service_ptr->client_id,
 				 service_ptr->service_use_count,
-				 service_ptr->service_use_count ==
-					 vc_use_count ? "" : " (+ more)");
+				 (service_ptr->service_use_count == vc_use_count)
+				 ? ""
+				 : " (+ more)");
 			break;
 		}
 	}
@@ -2192,22 +2199,26 @@ vchiq_arm_force_suspend(VCHIQ_STATE_T *state)
 		 * for the timeout */
 		stop_suspend_timer(arm_state);
 		if (!vchiq_videocore_wanted(state)) {
-			vchiq_log_info(vchiq_susp_log_level, "%s videocore "
-				"idle, initiating suspend", __func__);
+			vchiq_log_info(vchiq_susp_log_level,
+				       "%s videocore idle, initiating suspend",
+				       __func__);
 			status = vchiq_arm_vcsuspend(state);
 		} else if (arm_state->autosuspend_override <
 						FORCE_SUSPEND_FAIL_MAX) {
-			vchiq_log_info(vchiq_susp_log_level, "%s letting "
-				"videocore go idle", __func__);
+			vchiq_log_info(vchiq_susp_log_level,
+				       "%s letting videocore go idle",
+				       __func__);
 			status = VCHIQ_SUCCESS;
 		} else {
-			vchiq_log_warning(vchiq_susp_log_level, "%s failed too "
-				"many times - attempting suspend", __func__);
+			vchiq_log_warning(vchiq_susp_log_level,
+					  "%s failed too many times - attempting suspend",
+					  __func__);
 			status = vchiq_arm_vcsuspend(state);
 		}
 	} else {
-		vchiq_log_info(vchiq_susp_log_level, "%s videocore suspend "
-			"in progress - wait for completion", __func__);
+		vchiq_log_info(vchiq_susp_log_level,
+			       "%s videocore suspend in progress - wait for completion",
+			       __func__);
 		status = VCHIQ_SUCCESS;
 	}
 
@@ -2224,8 +2235,9 @@ vchiq_arm_force_suspend(VCHIQ_STATE_T *state)
 
 		write_lock_bh(&arm_state->susp_res_lock);
 		if (rc < 0) {
-			vchiq_log_warning(vchiq_susp_log_level, "%s "
-				"interrupted waiting for suspend", __func__);
+			vchiq_log_warning(vchiq_susp_log_level,
+					  "%s interrupted waiting for suspend",
+					  __func__);
 			status = VCHIQ_ERROR;
 			goto unblock_resume;
 		} else if (rc == 0) {
@@ -2434,8 +2446,9 @@ vchiq_use_internal(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service,
 			service->client_id);
 		entity_uc = &service->service_use_count;
 	} else {
-		vchiq_log_error(vchiq_susp_log_level, "%s null service "
-				"ptr", __func__);
+		vchiq_log_error(vchiq_susp_log_level,
+				"%s null service ptr",
+				__func__);
 		ret = VCHIQ_ERROR;
 		goto out;
 	}
@@ -2461,21 +2474,23 @@ vchiq_use_internal(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service,
 			 * continue */
 			arm_state->blocked_count++;
 			write_unlock_bh(&arm_state->susp_res_lock);
-			vchiq_log_info(vchiq_susp_log_level, "%s %s resume "
-				"blocked - waiting...", __func__, entity);
+			vchiq_log_info(vchiq_susp_log_level,
+				       "%s %s resume blocked - waiting...",
+				       __func__, entity);
 			if (wait_for_completion_killable(
 					&arm_state->resume_blocker) != 0) {
-				vchiq_log_error(vchiq_susp_log_level, "%s %s "
-					"wait for resume blocker interrupted",
-					__func__, entity);
+				vchiq_log_error(vchiq_susp_log_level,
+						"%s %s wait for resume blocker interrupted",
+						__func__, entity);
 				ret = VCHIQ_ERROR;
 				write_lock_bh(&arm_state->susp_res_lock);
 				arm_state->blocked_count--;
 				write_unlock_bh(&arm_state->susp_res_lock);
 				goto out;
 			}
-			vchiq_log_info(vchiq_susp_log_level, "%s %s resume "
-				"unblocked", __func__, entity);
+			vchiq_log_info(vchiq_susp_log_level,
+				       "%s %s resume unblocked",
+				       __func__, entity);
 			write_lock_bh(&arm_state->susp_res_lock);
 			if (--arm_state->blocked_count == 0)
 				complete_all(&arm_state->blocked_blocker);
@@ -2515,8 +2530,9 @@ vchiq_use_internal(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service,
 			__func__, entity);
 		if (wait_for_completion_killable(
 				&arm_state->vc_resume_complete) != 0) {
-			vchiq_log_error(vchiq_susp_log_level, "%s %s wait for "
-				"resume interrupted", __func__, entity);
+			vchiq_log_error(vchiq_susp_log_level,
+					"%s %s wait for resume interrupted",
+					__func__, entity);
 			ret = VCHIQ_ERROR;
 			goto out;
 		}
@@ -2793,10 +2809,9 @@ vchiq_dump_service_use_state(VCHIQ_STATE_T *state)
 		resume_state_names[vc_resume_state + VC_RESUME_NUM_OFFSET]);
 
 	if (only_nonzero)
-		vchiq_log_warning(vchiq_susp_log_level, "Too many active "
-			"services (%d).  Only dumping up to first %d services "
-			"with non-zero use-count", active_services,
-			local_max_services);
+		vchiq_log_warning(vchiq_susp_log_level,
+				  "Too many active services (%d). Only dumping up to first %d services with non-zero use-count",
+				  active_services, local_max_services);
 
 	for (i = 0; i < j; i++) {
 		vchiq_log_warning(vchiq_susp_log_level,
@@ -2834,13 +2849,14 @@ vchiq_check_service(VCHIQ_SERVICE_T *service)
 
 	if (ret == VCHIQ_ERROR) {
 		vchiq_log_error(vchiq_susp_log_level,
-			"%s ERROR - %c%c%c%c:%d service count %d, "
-			"state count %d, videocore suspend state %s", __func__,
-			VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc),
-			service->client_id, service->service_use_count,
-			arm_state->videocore_use_count,
-			suspend_state_names[arm_state->vc_suspend_state +
-						VC_SUSPEND_NUM_OFFSET]);
+				"%s ERROR - %c%c%c%c:%d service count %d, state count %d, videocore suspend state %s",
+				__func__,
+				VCHIQ_FOURCC_AS_4CHARS(service->base.fourcc),
+				service->client_id,
+				service->service_use_count,
+				arm_state->videocore_use_count,
+				suspend_state_names[arm_state->vc_suspend_state
+						    + VC_SUSPEND_NUM_OFFSET]);
 		vchiq_dump_service_use_state(service->state);
 	}
 out:
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/6] staging: vchiq_arm: Delete an unnecessary return statement in two functions
  2016-12-31 21:54 [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations SF Markus Elfring
                   ` (4 preceding siblings ...)
  2016-12-31 22:04 ` [PATCH 5/6] staging: vchiq_arm: Combine substrings for 24 messages SF Markus Elfring
@ 2016-12-31 22:06 ` SF Markus Elfring
  2017-01-03 17:46 ` [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations Eric Anholt
  6 siblings, 0 replies; 9+ messages in thread
From: SF Markus Elfring @ 2016-12-31 22:06 UTC (permalink / raw)
  To: linux-arm-kernel

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Dec 2016 22:05:19 +0100

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

WARNING: void function return statements are not generally useful

Thus remove such a statement in affected functions.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 05a00914dba0..6fe0c72246a2 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -2121,7 +2121,6 @@ vchiq_platform_check_suspend(VCHIQ_STATE_T *state)
 
 out:
 	vchiq_log_trace(vchiq_susp_log_level, "%s exit", __func__);
-	return;
 }
 
 
@@ -2306,7 +2305,6 @@ vchiq_check_suspend(VCHIQ_STATE_T *state)
 
 out:
 	vchiq_log_trace(vchiq_susp_log_level, "%s exit", __func__);
-	return;
 }
 
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations
  2016-12-31 21:54 [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations SF Markus Elfring
                   ` (5 preceding siblings ...)
  2016-12-31 22:06 ` [PATCH 6/6] staging: vchiq_arm: Delete an unnecessary return statement in two functions SF Markus Elfring
@ 2017-01-03 17:46 ` Eric Anholt
  2017-01-03 18:30   ` Greg Kroah-Hartman
  6 siblings, 1 reply; 9+ messages in thread
From: Eric Anholt @ 2017-01-03 17:46 UTC (permalink / raw)
  To: linux-arm-kernel

SF Markus Elfring <elfring@users.sourceforge.net> writes:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 31 Dec 2016 22:42:34 +0100
>
> Some update suggestions were taken into account
> from static source code analysis.

This series is:

Reviewed-by: Eric Anholt <eric@anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170103/9f31ad45/attachment.sig>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations
  2017-01-03 17:46 ` [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations Eric Anholt
@ 2017-01-03 18:30   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 9+ messages in thread
From: Greg Kroah-Hartman @ 2017-01-03 18:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 03, 2017 at 09:46:51AM -0800, Eric Anholt wrote:
> SF Markus Elfring <elfring@users.sourceforge.net> writes:
> 
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sat, 31 Dec 2016 22:42:34 +0100
> >
> > Some update suggestions were taken into account
> > from static source code analysis.
> 
> This series is:
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>

Nice, but I have a blacklist from this patch author, and have told them
numerous times that I no longer accept patches from them.  If you think
they are worthy, please resend them with your reviewed-by on them, but I
will warn you, I've blacklisted them for a reason...

good luck!

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-01-03 18:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-31 21:54 [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations SF Markus Elfring
2016-12-31 21:58 ` [PATCH 1/6] staging: vchiq_arm: Use kmalloc_array() in dump_phys_mem() SF Markus Elfring
2016-12-31 22:00 ` [PATCH 2/6] staging: vchiq_arm: Adjust 13 checks for null pointers SF Markus Elfring
2016-12-31 22:01 ` [PATCH 3/6] staging: vchiq_arm: One check less in dump_phys_mem() after error detection SF Markus Elfring
2016-12-31 22:03 ` [PATCH 4/6] staging: vchiq_arm: Delete an error message for a failed memory allocation in dump_phys_mem() SF Markus Elfring
2016-12-31 22:04 ` [PATCH 5/6] staging: vchiq_arm: Combine substrings for 24 messages SF Markus Elfring
2016-12-31 22:06 ` [PATCH 6/6] staging: vchiq_arm: Delete an unnecessary return statement in two functions SF Markus Elfring
2017-01-03 17:46 ` [PATCH 0/6] staging: vchiq_arm: Fine-tuning for some function implementations Eric Anholt
2017-01-03 18:30   ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).