[PATCH 0/9] memstick: Fine-tuning for some function implementations

[PATCH 0/9] memstick: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 20:44:20 +0100

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

Markus Elfring (9):
  Split a condition check in msb_ftl_initialize()
  Use kmalloc_array() in msb_ftl_initialize()
  Delete unwanted spaces behind three function parameters
  Improve two size determinations in msb_resume()
  Delete an unnecessary variable initialisation in msb_resume()
  Improve a size determination in msb_probe()
  Improve another size determination in msb_read_boot_blocks()
  Delete an unnecessary variable initialisation in msb_read_boot_blocks()
  Delete an unnecessary check in msb_update_block()

 drivers/memstick/core/ms_block.c | 57 ++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 25 deletions(-)

-- 
2.11.0

[PATCH 1/9] memstick: Split a condition check in msb_ftl_initialize()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 08:43:50 +0100

The functions "kmalloc" and "kzalloc" were called in two cases by the
function "msb_ftl_initialize" without checking immediately
if they succeded.
This issue was detected by using the Coccinelle software.

Split a condition check for memory allocation failures so that
the corresponding exception handling will be improved a bit.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/memstick/core/ms_block.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index f3512404bc52..fd8b1697a5a2 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -1339,17 +1339,17 @@ static int msb_ftl_initialize(struct msb_data *msb)
 	msb->logical_block_count = msb->zone_count * 496 - 2;
 
 	msb->used_blocks_bitmap = kzalloc(msb->block_count / 8, GFP_KERNEL);
+	if (!msb->used_blocks_bitmap)
+		return -ENOMEM;
+
 	msb->erased_blocks_bitmap = kzalloc(msb->block_count / 8, GFP_KERNEL);
+	if (!msb->erased_blocks_bitmap)
+		goto free_used_bitmap;
+
 	msb->lba_to_pba_table =
 		kmalloc(msb->logical_block_count * sizeof(u16), GFP_KERNEL);
-
-	if (!msb->used_blocks_bitmap || !msb->lba_to_pba_table ||
-						!msb->erased_blocks_bitmap) {
-		kfree(msb->used_blocks_bitmap);
-		kfree(msb->lba_to_pba_table);
-		kfree(msb->erased_blocks_bitmap);
-		return -ENOMEM;
-	}
+	if (!msb->lba_to_pba_table)
+		goto free_erased_bitmap;
 
 	for (i = 0; i < msb->zone_count; i++)
 		msb->free_block_count[i] = MS_BLOCKS_IN_ZONE;
@@ -1362,6 +1362,11 @@ static int msb_ftl_initialize(struct msb_data *msb)
 
 	msb->ftl_initialized = true;
 	return 0;
+free_erased_bitmap:
+	kfree(msb->erased_blocks_bitmap);
+free_used_bitmap:
+	kfree(msb->used_blocks_bitmap);
+	return -ENOMEM;
 }
 
 static int msb_ftl_scan(struct msb_data *msb)
-- 
2.11.0

[PATCH 2/9] memstick: Use kmalloc_array() in msb_ftl_initialize()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 09:15:27 +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/memstick/core/ms_block.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index fd8b1697a5a2..ec17885a5956 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -1346,8 +1346,9 @@ static int msb_ftl_initialize(struct msb_data *msb)
 	if (!msb->erased_blocks_bitmap)
 		goto free_used_bitmap;
 
-	msb->lba_to_pba_table =
-		kmalloc(msb->logical_block_count * sizeof(u16), GFP_KERNEL);
+	msb->lba_to_pba_table = kmalloc_array(msb->logical_block_count,
+					      sizeof(*msb->lba_to_pba_table),
+					      GFP_KERNEL);
 	if (!msb->lba_to_pba_table)
 		goto free_erased_bitmap;
 
-- 
2.11.0

[PATCH 3/9] memstick: Delete unwanted spaces behind three function parameters

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 18:54:25 +0100

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

ERROR: space prohibited before that ',' (ctx:WxW)

Thus fix the affected source code places.

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

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index ec17885a5956..526e671a3435 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -194,7 +194,8 @@ static void msb_mark_block_unused(struct msb_data *msb, int pba)
 	int zone = msb_get_zone_from_pba(pba);
 
 	if (!test_bit(pba, msb->used_blocks_bitmap)) {
-		pr_err("BUG: attempt to mark already unused pba %d as unused" , pba);
+		pr_err("BUG: attempt to mark already unused pba %d as unused",
+		       pba);
 		msb->read_only = true;
 		return;
 	}
@@ -1551,7 +1552,7 @@ static int msb_cache_flush(struct msb_data *msb)
 	dbg_verbose("Flushing the write cache of pba %d (LBA %d)",
 						pba, msb->cache_block_lba);
 
-	sg_init_one(&sg, msb->cache , msb->block_size);
+	sg_init_one(&sg, msb->cache, msb->block_size);
 
 	/* Read all missing pages in cache */
 	for (page = 0; page < msb->pages_in_block; page++) {
@@ -1597,7 +1598,9 @@ static int msb_cache_flush(struct msb_data *msb)
 			dbg("marking page %d as containing damaged data",
 				page);
 			msb_set_overwrite_flag(msb,
-				pba , page, 0xFF & ~MEMSTICK_OV_PG_NORMAL);
+					       pba,
+					       page,
+					       0xFF & ~MEMSTICK_OV_PG_NORMAL);
 		}
 	}
 
-- 
2.11.0

[PATCH 4/9] memstick: Improve two size determinations in msb_resume()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 19:17:07 +0100

Replace the specification of data structures by pointer dereferences
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/memstick/core/ms_block.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index 526e671a3435..fd3d9f78bcba 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -2261,8 +2261,7 @@ static int msb_resume(struct memstick_dev *card)
 	return 0;
 #endif
 	mutex_lock(&card->host->lock);
-
-	new_msb = kzalloc(sizeof(struct msb_data), GFP_KERNEL);
+	new_msb = kzalloc(sizeof(*new_msb), GFP_KERNEL);
 	if (!new_msb)
 		goto out;
 
@@ -2277,8 +2276,9 @@ static int msb_resume(struct memstick_dev *card)
 	if (msb->block_size != new_msb->block_size)
 		goto out;
 
-	if (memcmp(msb->boot_page, new_msb->boot_page,
-					sizeof(struct ms_boot_page)))
+	if (memcmp(msb->boot_page,
+		   new_msb->boot_page,
+		   sizeof(*msb->boot_page)))
 		goto out;
 
 	if (msb->logical_block_count != new_msb->logical_block_count ||
-- 
2.11.0

[PATCH 5/9] memstick: Delete an unnecessary variable initialisation in msb_resume()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 19:29:17 +0100

The local variable "new_msb" will eventually be set to an appropriate
pointer a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/memstick/core/ms_block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index fd3d9f78bcba..818c8f8de553 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -2253,7 +2253,7 @@ static int msb_suspend(struct memstick_dev *card, pm_message_t state)
 static int msb_resume(struct memstick_dev *card)
 {
 	struct msb_data *msb = memstick_get_drvdata(card);
-	struct msb_data *new_msb = NULL;
+	struct msb_data *new_msb;
 	bool card_dead = true;
 
 #ifndef CONFIG_MEMSTICK_UNSAFE_RESUME
-- 
2.11.0

[PATCH 6/9] memstick: Improve a size determination in msb_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 19:36:21 +0100

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/memstick/core/ms_block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index 818c8f8de553..a30c0ae95ae5 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -2188,7 +2188,7 @@ static int msb_probe(struct memstick_dev *card)
 	struct msb_data *msb;
 	int rc = 0;
 
-	msb = kzalloc(sizeof(struct msb_data), GFP_KERNEL);
+	msb = kzalloc(sizeof(*msb), GFP_KERNEL);
 	if (!msb)
 		return -ENOMEM;
 	memstick_set_drvdata(card, msb);
-- 
2.11.0

[PATCH 7/9] memstick: Improve another size determination in msb_read_boot_blocks()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 20:00:16 +0100

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/memstick/core/ms_block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index a30c0ae95ae5..98ab5764050b 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -1202,7 +1202,7 @@ static int msb_read_boot_blocks(struct msb_data *msb)
 	dbg_verbose("Start of a scan for the boot blocks");
 
 	if (!msb->boot_page) {
-		page = kmalloc(sizeof(struct ms_boot_page)*2, GFP_KERNEL);
+		page = kmalloc(sizeof(*page) * 2, GFP_KERNEL);
 		if (!page)
 			return -ENOMEM;
 
-- 
2.11.0

[PATCH 7/9] memstick: Improve another size determination in msb_read_boot_blocks()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 20:00:16 +0100

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/memstick/core/ms_block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index a30c0ae95ae5..98ab5764050b 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -1202,7 +1202,7 @@ static int msb_read_boot_blocks(struct msb_data *msb)
 	dbg_verbose("Start of a scan for the boot blocks");
 
 	if (!msb->boot_page) {
-		page = kmalloc(sizeof(struct ms_boot_page)*2, GFP_KERNEL);
+		page = kmalloc(sizeof(*page) * 2, GFP_KERNEL);
 		if (!page)
 			return -ENOMEM;
 
-- 
2.11.0

[PATCH 8/9] memstick: Delete an unnecessary variable initialisation in msb_read_boot_blocks()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 20:02:24 +0100

The local variable "pba" will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/memstick/core/ms_block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index 98ab5764050b..861f6a76136d 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -1190,7 +1190,7 @@ static void msb_fix_boot_page_endianness(struct ms_boot_page *p)
 
 static int msb_read_boot_blocks(struct msb_data *msb)
 {
-	int pba = 0;
+	int pba;
 	struct scatterlist sg;
 	struct ms_extra_data_register extra;
 	struct ms_boot_page *page;
-- 
2.11.0

[PATCH 9/9] memstick: Delete an unnecessary check in msb_update_block()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 7 Jan 2017 20:21:02 +0100

Further exception handling will only be performed after an error code
was set. Thus remove a redundant check for a local variable at the end.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/memstick/core/ms_block.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index 861f6a76136d..013f317d566b 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -1154,10 +1154,8 @@ static int msb_update_block(struct msb_data *msb, u16 lba,
 		return 0;
 	}
 out:
-	if (error) {
-		pr_err("block update error after %d tries,  switching to r/o mode", try);
-		msb->read_only = true;
-	}
+	pr_err("block update error after %d tries, switching to r/o mode", try);
+	msb->read_only = true;
 	return error;
 }
 
-- 
2.11.0