[Qemu-devel] [PATCH 0/6] firmware-qemu_fw_cfg: Fine-tuning for four function implementations

[Qemu-devel] [PATCH 0/6] firmware-qemu_fw_cfg: Fine-tuning for four function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Sep 2016 14:43:21 +0200

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

Markus Elfring (6):
  Use kmalloc_array() in fw_cfg_register_dir_entries()
  Improve a size determination in fw_cfg_register_file()
  Rename jump labels in fw_cfg_register_file()
  Improve a size determination in fw_cfg_build_symlink()
  Rename jump labels in fw_cfg_sysfs_probe()
  Move a variable assignment in fw_cfg_sysfs_probe()

 drivers/firmware/qemu_fw_cfg.c | 53 +++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

-- 
2.10.0

[Qemu-devel] [PATCH 1/6] firmware-qemu_fw_cfg: Use kmalloc_array() in fw_cfg_register_dir_entries()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Sep 2016 09:39:31 +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.

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

* Delete the local variable "dir_size" which became unnecessary with
  this refactoring.

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

diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index 0e20116..e69653e 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -491,17 +491,17 @@ static int fw_cfg_register_dir_entries(void)
 	int ret = 0;
 	u32 count, i;
 	struct fw_cfg_file *dir;
-	size_t dir_size;
 
 	fw_cfg_read_blob(FW_CFG_FILE_DIR, &count, 0, sizeof(count));
 	count = be32_to_cpu(count);
-	dir_size = count * sizeof(struct fw_cfg_file);
-
-	dir = kmalloc(dir_size, GFP_KERNEL);
+	dir = kmalloc_array(count, sizeof(*dir), GFP_KERNEL);
 	if (!dir)
 		return -ENOMEM;
 
-	fw_cfg_read_blob(FW_CFG_FILE_DIR, dir, sizeof(count), dir_size);
+	fw_cfg_read_blob(FW_CFG_FILE_DIR,
+			 dir,
+			 sizeof(count),
+			 sizeof(*dir) * count);
 
 	for (i = 0; i < count; i++) {
 		dir[i].size = be32_to_cpu(dir[i].size);
-- 
2.10.0

[Qemu-devel] [PATCH 2/6] firmware-qemu_fw_cfg: Improve a size determination in fw_cfg_register_file()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Sep 2016 10:43:27 +0200

Replace the specification of a data structure by a pointer dereference
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/firmware/qemu_fw_cfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index e69653e..a229df6 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -458,7 +458,7 @@ static int fw_cfg_register_file(const struct fw_cfg_file *f)
 		return -ENOMEM;
 
 	/* set file entry information */
-	memcpy(&entry->f, f, sizeof(struct fw_cfg_file));
+	memcpy(&entry->f, f, sizeof(*f));
 
 	/* register entry under "/sys/firmware/qemu_fw_cfg/by_key/" */
 	err = kobject_init_and_add(&entry->kobj, &fw_cfg_sysfs_entry_ktype,
-- 
2.10.0

[Qemu-devel] [PATCH 3/6] firmware-qemu_fw_cfg: Rename jump labels in fw_cfg_register_file()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Sep 2016 11:23:46 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/firmware/qemu_fw_cfg.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index a229df6..a834d01 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -464,12 +464,12 @@ static int fw_cfg_register_file(const struct fw_cfg_file *f)
 	err = kobject_init_and_add(&entry->kobj, &fw_cfg_sysfs_entry_ktype,
 				   fw_cfg_sel_ko, "%d", entry->f.select);
 	if (err)
-		goto err_register;
+		goto free_entry;
 
 	/* add raw binary content access */
 	err = sysfs_create_bin_file(&entry->kobj, &fw_cfg_sysfs_attr_raw);
 	if (err)
-		goto err_add_raw;
+		goto delete_object;
 
 	/* try adding "/sys/firmware/qemu_fw_cfg/by_name/" symlink */
 	fw_cfg_build_symlink(fw_cfg_fname_kset, &entry->kobj, entry->f.name);
@@ -477,10 +477,9 @@ static int fw_cfg_register_file(const struct fw_cfg_file *f)
 	/* success, add entry to global cache */
 	fw_cfg_sysfs_cache_enlist(entry);
 	return 0;
-
-err_add_raw:
+ delete_object:
 	kobject_del(&entry->kobj);
-err_register:
+ free_entry:
 	kfree(entry);
 	return err;
 }
-- 
2.10.0

[Qemu-devel] [PATCH 4/6] firmware-qemu_fw_cfg: Improve a size determination in fw_cfg_build_symlink()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Sep 2016 14:02:02 +0200

Replace the specification of a data structure by a pointer dereference
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/firmware/qemu_fw_cfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index a834d01..a4b108a 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -399,7 +399,7 @@ static int fw_cfg_build_symlink(struct kset *dir,
 			dir = to_kset(ko);
 		} else {
 			/* create new subdirectory kset */
-			subdir = kzalloc(sizeof(struct kset), GFP_KERNEL);
+			subdir = kzalloc(sizeof(*subdir), GFP_KERNEL);
 			if (!subdir) {
 				ret = -ENOMEM;
 				break;
-- 
2.10.0

[Qemu-devel] [PATCH 5/6] firmware-qemu_fw_cfg: Rename jump labels in fw_cfg_sysfs_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Sep 2016 14:04:48 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/firmware/qemu_fw_cfg.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index a4b108a..a09e59c 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -538,42 +538,41 @@ static int fw_cfg_sysfs_probe(struct platform_device *pdev)
 	err = -ENOMEM;
 	fw_cfg_sel_ko = kobject_create_and_add("by_key", fw_cfg_top_ko);
 	if (!fw_cfg_sel_ko)
-		goto err_sel;
+		goto exit;
 	fw_cfg_fname_kset = kset_create_and_add("by_name", NULL, fw_cfg_top_ko);
 	if (!fw_cfg_fname_kset)
-		goto err_name;
+		goto cleanup_object;
 
 	/* initialize fw_cfg device i/o from platform data */
 	err = fw_cfg_do_platform_probe(pdev);
 	if (err)
-		goto err_probe;
+		goto unregister;
 
 	/* get revision number, add matching top-level attribute */
 	fw_cfg_read_blob(FW_CFG_ID, &fw_cfg_rev, 0, sizeof(fw_cfg_rev));
 	fw_cfg_rev = le32_to_cpu(fw_cfg_rev);
 	err = sysfs_create_file(fw_cfg_top_ko, &fw_cfg_rev_attr.attr);
 	if (err)
-		goto err_rev;
+		goto cleanup_io;
 
 	/* process fw_cfg file directory entry, registering each file */
 	err = fw_cfg_register_dir_entries();
 	if (err)
-		goto err_dir;
+		goto cleanup_cache;
 
 	/* success */
 	pr_debug("fw_cfg: loaded.\n");
 	return 0;
-
-err_dir:
+ cleanup_cache:
 	fw_cfg_sysfs_cache_cleanup();
 	sysfs_remove_file(fw_cfg_top_ko, &fw_cfg_rev_attr.attr);
-err_rev:
+ cleanup_io:
 	fw_cfg_io_cleanup();
-err_probe:
+ unregister:
 	fw_cfg_kset_unregister_recursive(fw_cfg_fname_kset);
-err_name:
+ cleanup_object:
 	fw_cfg_kobj_cleanup(fw_cfg_sel_ko);
-err_sel:
+ exit:
 	return err;
 }
 
-- 
2.10.0

[Qemu-devel] [PATCH 6/6] firmware-qemu_fw_cfg: Move a variable assignment in fw_cfg_sysfs_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 18 Sep 2016 14:25:55 +0200

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

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

diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
index a09e59c..23293a1 100644
--- a/drivers/firmware/qemu_fw_cfg.c
+++ b/drivers/firmware/qemu_fw_cfg.c
@@ -535,13 +535,16 @@ static int fw_cfg_sysfs_probe(struct platform_device *pdev)
 		return -EBUSY;
 
 	/* create by_key and by_name subdirs of /sys/firmware/qemu_fw_cfg/ */
-	err = -ENOMEM;
 	fw_cfg_sel_ko = kobject_create_and_add("by_key", fw_cfg_top_ko);
-	if (!fw_cfg_sel_ko)
+	if (!fw_cfg_sel_ko) {
+		err = -ENOMEM;
 		goto exit;
+	}
 	fw_cfg_fname_kset = kset_create_and_add("by_name", NULL, fw_cfg_top_ko);
-	if (!fw_cfg_fname_kset)
+	if (!fw_cfg_fname_kset) {
+		err = -ENOMEM;
 		goto cleanup_object;
+	}
 
 	/* initialize fw_cfg device i/o from platform data */
 	err = fw_cfg_do_platform_probe(pdev);
-- 
2.10.0

Re: [Qemu-devel] [PATCH 0/6] firmware-qemu_fw_cfg: Fine-tuning for four function implementations

[Gabriel L. Somlo]

On Sun, Sep 18, 2016 at 02:48:30PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 18 Sep 2016 14:43:21 +0200
> 
> Some update suggestions were taken into account
> from static source code analysis.
> 
> Markus Elfring (6):
>   Use kmalloc_array() in fw_cfg_register_dir_entries()
>   Improve a size determination in fw_cfg_register_file()
>   Rename jump labels in fw_cfg_register_file()
>   Improve a size determination in fw_cfg_build_symlink()
>   Rename jump labels in fw_cfg_sysfs_probe()
>   Move a variable assignment in fw_cfg_sysfs_probe()

Acked-by: Gabriel Somlo <somlo@cmu.edu>

>  drivers/firmware/qemu_fw_cfg.c | 53 +++++++++++++++++++++---------------------
>  1 file changed, 27 insertions(+), 26 deletions(-)
> 
> -- 
> 2.10.0
>