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

Groups > linux.kernel > #1707415 > unrolled thread

[PATCH v2 0/9] ACPI-video: Fine-tuning for several function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 0/9] ACPI-video: Fine-tuning for several function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:40 +0200
    [PATCH v2 1/9] ACPI-video: Use kmalloc_array() in  acpi_video_get_levels() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:40 +0200
    [PATCH v2 3/9] ACPI-video: Delete an error message for a failed  kzalloc() call SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:40 +0200
    [PATCH v2 2/9] ACPI-video: Return directly after a failed device  query SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:40 +0200
    [PATCH v2 9/9] ACPI-video: Improve a size determination in four  functions SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:50 +0200
    [PATCH v2 8/9] ACPI-video: Move four assignments in  acpi_video_get_levels() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:50 +0200
    [PATCH v2 4/9] ACPI-video: Return directly after a failed  input_allocate_device() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:50 +0200
    [PATCH v2 6/9] ACPI-video: Improve a jump target in  acpi_video_switch_brightness() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:50 +0200
    [PATCH v2 7/9] ACPI-video: Delete an unnecessary initialisation in  three functions SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:50 +0200
    [PATCH v2 5/9] ACPI-video: Delete unnecessary if statement in  acpi_video_switch_brightness() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-09 17:50 +0200

#1707415 — [PATCH v2 0/9] ACPI-video: Fine-tuning for several function implementations

From 2a7010a6555091b685ed51d91fea23ff9058047c Mon Sep 17 00:00:00 2001
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 17:16:15 +0200
Subject: [PATCH v2 0/9] ACPI-video: Fine-tuning for several function implementations

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

Markus Elfring (9):
  Use kmalloc_array() in acpi_video_get_levels()
  Return directly after a failed device query
  Delete an error message for a failed kzalloc() call
  Return directly after a failed input_allocate_device()
  Delete unnecessary if statement in acpi_video_switch_brightness()
  Improve a jump target in acpi_video_switch_brightness()
  Delete an unnecessary initialisation in three functions
  Move four assignments in acpi_video_get_levels()
  Improve a size determination in four functions

---

v2:
* Desired changes were rebased on source files from Linux next-20170803.

* Some update steps were recombined.

 drivers/acpi/acpi_video.c | 53 ++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 28 deletions(-)

-- 
2.13.4

[toc] | [next] | [standalone]

#1707416 — [PATCH v2 1/9] ACPI-video: Use kmalloc_array() in acpi_video_get_levels()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 14:52:25 +0200

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.

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

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 0972ec0e2eb8..1aaf2b591ca8 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -832,8 +832,9 @@ int acpi_video_get_levels(struct acpi_device *device,
 	 * in order to account for buggy BIOS which don't export the first two
 	 * special levels (see below)
 	 */
-	br->levels = kmalloc((obj->package.count + ACPI_VIDEO_FIRST_LEVEL) *
-	                     sizeof(*br->levels), GFP_KERNEL);
+	br->levels = kmalloc_array(obj->package.count + ACPI_VIDEO_FIRST_LEVEL,
+				   sizeof(*br->levels),
+				   GFP_KERNEL);
 	if (!br->levels) {
 		result = -ENOMEM;
 		goto out_free;
-- 
2.13.4

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

#1707417 — [PATCH v2 3/9] ACPI-video: Delete an error message for a failed kzalloc() call

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 15:07:53 +0200

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/acpi/acpi_video.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 7a9cda6c0a6b..f83ece9c830c 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -821,7 +821,6 @@ int acpi_video_get_levels(struct acpi_device *device,
 
 	br = kzalloc(sizeof(*br), GFP_KERNEL);
 	if (!br) {
-		printk(KERN_ERR "can't allocate memory\n");
 		result = -ENOMEM;
 		goto out;
 	}
-- 
2.13.4

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

#1707418 — [PATCH v2 2/9] ACPI-video: Return directly after a failed device query

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 15:00:45 +0200

Return directly after a function call "acpi_video_device_lcd_query_levels"
failed at the beginning.

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

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 1aaf2b591ca8..7a9cda6c0a6b 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -811,8 +811,7 @@ int acpi_video_get_levels(struct acpi_device *device,
 								&obj))) {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Could not query available "
 						"LCD brightness level\n"));
-		result = -ENODEV;
-		goto out;
+		return -ENODEV;
 	}
 
 	if (obj->package.count < ACPI_VIDEO_FIRST_LEVEL) {
-- 
2.13.4

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

#1707422 — [PATCH v2 9/9] ACPI-video: Improve a size determination in four functions

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 16:45:29 +0200

Replace the specification of a data structures by variable references
as the parameter for the operator "sizeof" 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/acpi/acpi_video.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 3cc1798676ab..d999f4e49b98 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1140,7 +1140,7 @@ acpi_video_bus_get_one_device(struct acpi_device *device,
 	if (ACPI_FAILURE(status))
 		return 0;
 
-	data = kzalloc(sizeof(struct acpi_video_device), GFP_KERNEL);
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 
@@ -1321,7 +1321,7 @@ static int acpi_video_device_enumerate(struct acpi_video_bus *video)
 			  dod->package.count));
 
 	active_list = kcalloc(1 + dod->package.count,
-			      sizeof(struct acpi_video_enumerated_device),
+			      sizeof(*active_list),
 			      GFP_KERNEL);
 	if (!active_list) {
 		status = -ENOMEM;
@@ -1742,7 +1742,7 @@ static void acpi_video_dev_register_backlight(struct acpi_video_device *device)
 		pci_dev_put(pdev);
 	}
 
-	memset(&props, 0, sizeof(struct backlight_properties));
+	memset(&props, 0, sizeof(props));
 	props.type = BACKLIGHT_FIRMWARE;
 	props.max_brightness =
 		device->brightness->count - ACPI_VIDEO_FIRST_LEVEL - 1;
@@ -2007,7 +2007,7 @@ static int acpi_video_bus_add(struct acpi_device *device)
 			return -ENODEV;
 	}
 
-	video = kzalloc(sizeof(struct acpi_video_bus), GFP_KERNEL);
+	video = kzalloc(sizeof(*video), GFP_KERNEL);
 	if (!video)
 		return -ENOMEM;
 
-- 
2.13.4

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

#1707424 — [PATCH v2 8/9] ACPI-video: Move four assignments in acpi_video_get_levels()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 16:21:17 +0200

Move the assignments for four local variables so that they will only
be performed if the corresponding data processing succeeded
by this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/acpi/acpi_video.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index a899faf6ff84..3cc1798676ab 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -801,10 +801,10 @@ int acpi_video_get_levels(struct acpi_device *device,
 			  int *pmax_level)
 {
 	union acpi_object *obj = NULL;
-	int i, max_level = 0, count = 0, level_ac_battery = 0;
+	int i, max_level, count, level_ac_battery;
 	union acpi_object *o;
 	struct acpi_video_device_brightness *br;
-	int result = 0;
+	int result;
 	u32 value;
 
 	if (!ACPI_SUCCESS(acpi_video_device_lcd_query_levels(device->handle,
@@ -838,6 +838,8 @@ int acpi_video_get_levels(struct acpi_device *device,
 		goto out_free;
 	}
 
+	max_level = 0;
+	count = 0;
 	for (i = 0; i < obj->package.count; i++) {
 		o = (union acpi_object *)&obj->package.elements[i];
 		if (o->type != ACPI_TYPE_INTEGER) {
@@ -863,6 +865,7 @@ int acpi_video_get_levels(struct acpi_device *device,
 	 * In this case, the first two elements in _BCL packages
 	 * are also supported brightness levels that OS should take care of.
 	 */
+	level_ac_battery = 0;
 	for (i = ACPI_VIDEO_FIRST_LEVEL; i < count; i++) {
 		if (br->levels[i] == br->levels[ACPI_VIDEO_AC_LEVEL])
 			level_ac_battery++;
@@ -895,7 +898,7 @@ int acpi_video_get_levels(struct acpi_device *device,
 	*dev_br = br;
 	if (pmax_level)
 		*pmax_level = max_level;
-
+	result = 0;
 out:
 	kfree(obj);
 	return result;
-- 
2.13.4

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

#1707425 — [PATCH v2 4/9] ACPI-video: Return directly after a failed input_allocate_device()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 15:15:16 +0200

* Return directly after a call of the function "input_allocate_device"
  failed at the beginning.

* Delete the jump label "out" which became unnecessary with
  this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/acpi/acpi_video.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index f83ece9c830c..09ea1f115f55 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1903,10 +1903,8 @@ static int acpi_video_bus_add_notify_handler(struct acpi_video_bus *video)
 	int error;
 
 	video->input = input = input_allocate_device();
-	if (!input) {
-		error = -ENOMEM;
-		goto out;
-	}
+	if (!input)
+		return -ENOMEM;
 
 	error = acpi_video_bus_start_devices(video);
 	if (error)
@@ -1946,7 +1944,6 @@ static int acpi_video_bus_add_notify_handler(struct acpi_video_bus *video)
 err_free_input:
 	input_free_device(input);
 	video->input = NULL;
-out:
 	return error;
 }
 
-- 
2.13.4

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

#1707428 — [PATCH v2 6/9] ACPI-video: Improve a jump target in acpi_video_switch_brightness()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 15:45:35 +0200

* Avoid another duplicate check for the local variable "result"
  then at the end.

* Jump directly to an error message in the case that the desired brightness
  can not be switched.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/acpi/acpi_video.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 8295ae1deab9..e279ed221961 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1411,19 +1411,19 @@ acpi_video_switch_brightness(struct work_struct *work)
 		return;
 
 	if (!device->brightness)
-		goto out;
+		goto report_failure;
 
 	result = acpi_video_device_lcd_get_level_current(device,
 							 &level_current,
 							 false);
 	if (result)
-		goto out;
+		goto report_failure;
 
 	level_next = acpi_video_get_next_level(device, level_current, event);
 
 	result = acpi_video_device_lcd_set_level(device, level_next);
-out:
 	if (result)
+ report_failure:
 		printk(KERN_ERR PREFIX "Failed to switch the brightness\n");
 	else
 		backlight_force_update(device->backlight,
-- 
2.13.4

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

#1707429 — [PATCH v2 7/9] ACPI-video: Delete an unnecessary initialisation in three functions

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 15:55:35 +0200

Three local variables will be set to appropriate values a bit later.
Thus omit the explicit initialisation at the beginning of these functions.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/acpi/acpi_video.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index e279ed221961..a899faf6ff84 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -803,7 +803,7 @@ int acpi_video_get_levels(struct acpi_device *device,
 	union acpi_object *obj = NULL;
 	int i, max_level = 0, count = 0, level_ac_battery = 0;
 	union acpi_object *o;
-	struct acpi_video_device_brightness *br = NULL;
+	struct acpi_video_device_brightness *br;
 	int result = 0;
 	u32 value;
 
@@ -921,7 +921,7 @@ acpi_video_init_brightness(struct acpi_video_device *device)
 	int i, max_level = 0;
 	unsigned long long level, level_old;
 	struct acpi_video_device_brightness *br = NULL;
-	int result = -EINVAL;
+	int result;
 
 	result = acpi_video_get_levels(device->dev, &br, &max_level);
 	if (result)
@@ -1295,7 +1295,7 @@ static int acpi_video_device_enumerate(struct acpi_video_bus *video)
 	int i;
 	struct acpi_video_enumerated_device *active_list;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	union acpi_object *dod = NULL;
+	union acpi_object *dod;
 	union acpi_object *obj;
 
 	if (!video->cap._DOD)
-- 
2.13.4

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

#1707432 — [PATCH v2 5/9] ACPI-video: Delete unnecessary if statement in acpi_video_switch_brightness()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 9 Aug 2017 15:43:17 +0200

Move a function call into an else branch for successful function execution.
Omit a duplicate check for the local variable "result" then at the end.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/acpi/acpi_video.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
index 09ea1f115f55..8295ae1deab9 100644
--- a/drivers/acpi/acpi_video.c
+++ b/drivers/acpi/acpi_video.c
@@ -1422,14 +1422,12 @@ acpi_video_switch_brightness(struct work_struct *work)
 	level_next = acpi_video_get_next_level(device, level_current, event);
 
 	result = acpi_video_device_lcd_set_level(device, level_next);
-
-	if (!result)
-		backlight_force_update(device->backlight,
-				       BACKLIGHT_UPDATE_HOTKEY);
-
 out:
 	if (result)
 		printk(KERN_ERR PREFIX "Failed to switch the brightness\n");
+	else
+		backlight_force_update(device->backlight,
+				       BACKLIGHT_UPDATE_HOTKEY);
 }
 
 int acpi_video_get_edid(struct acpi_device *device, int type, int device_id,
-- 
2.13.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web