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

Groups > linux.kernel > #1557318 > unrolled thread

[PATCH 0/3] MTD-FTL: Fine-tuning for two function implementations

Started bySF Markus Elfring <elfring@users.sourceforge.net>
First post2017-01-12 11:40 +0100
Last post2017-01-12 11:50 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] MTD-FTL: Fine-tuning for two function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-12 11:40 +0100
    [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-12 11:40 +0100
      Re: [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps() Cyrille Pitchen <cyrille.pitchen@atmel.com> - 2017-01-12 14:10 +0100
        Re: mtd/ftl: Use kmalloc_array() in build_maps() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-12 18:00 +0100
          Re: mtd/ftl: Use kmalloc_array() in build_maps() Marek Vasut <marek.vasut@gmail.com> - 2017-01-12 18:00 +0100
    [PATCH 2/3] mtd/ftl: Delete an error message for a failed memory  allocation in ftl_add_mtd() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-12 11:40 +0100
    [PATCH 3/3] mtd/ftl: Improve another size determination in  ftl_add_mtd() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-12 11:50 +0100

#1557318 — [PATCH 0/3] MTD-FTL: Fine-tuning for two function implementations

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-01-12 11:40 +0100
Subject[PATCH 0/3] MTD-FTL: Fine-tuning for two function implementations
Message-ID<sYKXw-2QD-37@gated-at.bofh.it>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 12 Jan 2017 11:24:12 +0100

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

Markus Elfring (3):
  Use kmalloc_array() in build_maps()
  Delete an error message for a failed memory allocation in ftl_add_mtd()
  Improve another size determination in ftl_add_mtd()

 drivers/mtd/ftl.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1557321 — [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-01-12 11:40 +0100
Subject[PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps()
Message-ID<sYKXw-2QD-39@gated-at.bofh.it>
In reply to#1557318
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 12 Jan 2017 10:42:25 +0100

* Multiplications for the size determination of memory allocations
  indicated that array data structures should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of data types by pointer dereferences
  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/mtd/ftl.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
index 9fb3b0dcdac2..ef2f38b6a837 100644
--- a/drivers/mtd/ftl.c
+++ b/drivers/mtd/ftl.c
@@ -207,15 +207,14 @@ static int build_maps(partition_t *part)
     /* Set up erase unit maps */
     part->DataUnits = le16_to_cpu(part->header.NumEraseUnits) -
 	part->header.NumTransferUnits;
-    part->EUNInfo = kmalloc(part->DataUnits * sizeof(struct eun_info_t),
-			    GFP_KERNEL);
+	part->EUNInfo = kmalloc_array(part->DataUnits, sizeof(*part->EUNInfo),
+				      GFP_KERNEL);
     if (!part->EUNInfo)
 	    goto out;
     for (i = 0; i < part->DataUnits; i++)
 	part->EUNInfo[i].Offset = 0xffffffff;
-    part->XferInfo =
-	kmalloc(part->header.NumTransferUnits * sizeof(struct xfer_info_t),
-		GFP_KERNEL);
+	part->XferInfo = kmalloc_array(part->header.NumTransferUnits,
+				       sizeof(*part->XferInfo), GFP_KERNEL);
     if (!part->XferInfo)
 	    goto out_EUNInfo;
 
@@ -275,8 +274,8 @@ static int build_maps(partition_t *part)
     memset(part->VirtualBlockMap, 0xff, blocks * sizeof(uint32_t));
     part->BlocksPerUnit = (1 << header.EraseUnitSize) >> header.BlockSize;
 
-    part->bam_cache = kmalloc(part->BlocksPerUnit * sizeof(uint32_t),
-			      GFP_KERNEL);
+	part->bam_cache = kmalloc_array(part->BlocksPerUnit,
+					sizeof(*part->bam_cache), GFP_KERNEL);
     if (!part->bam_cache)
 	    goto out_VirtualBlockMap;
 
-- 
2.11.0

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

#1557426 — Re: [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps()

FromCyrille Pitchen <cyrille.pitchen@atmel.com>
Date2017-01-12 14:10 +0100
SubjectRe: [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps()
Message-ID<sYNiF-4mX-5@gated-at.bofh.it>
In reply to#1557321
Le 12/01/2017 à 11:35, SF Markus Elfring a écrit :
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 12 Jan 2017 10:42:25 +0100
> 
> * Multiplications for the size determination of memory allocations
>   indicated that array data structures should be processed.
>   Thus use the corresponding function "kmalloc_array".
> 
>   This issue was detected by using the Coccinelle software.
> 
> * Replace the specification of data types by pointer dereferences
>   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/mtd/ftl.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
> index 9fb3b0dcdac2..ef2f38b6a837 100644
> --- a/drivers/mtd/ftl.c
> +++ b/drivers/mtd/ftl.c
> @@ -207,15 +207,14 @@ static int build_maps(partition_t *part)
>      /* Set up erase unit maps */
>      part->DataUnits = le16_to_cpu(part->header.NumEraseUnits) -
>  	part->header.NumTransferUnits;
> -    part->EUNInfo = kmalloc(part->DataUnits * sizeof(struct eun_info_t),
> -			    GFP_KERNEL);
> +	part->EUNInfo = kmalloc_array(part->DataUnits, sizeof(*part->EUNInfo),
> +				      GFP_KERNEL);

The indentation has been changed and the new one looks wrong...

I understand the original indentation, with spaces, doesn't follow the
Linux coding style but at least it's consistent and readable.

Your patch uses a different indentation with tabs, which now mixes two
different indentations: IMHO, this is worst than before.

If you want to fix the indentation to make it compliant with the Linux
coding style, do it on the whole file so every thing is uniform.

Reviewing such dummy/automatic patches is a pure waste of time, so
personally I think we should just ignore them.

>      if (!part->EUNInfo)
>  	    goto out;
>      for (i = 0; i < part->DataUnits; i++)
>  	part->EUNInfo[i].Offset = 0xffffffff;
> -    part->XferInfo =
> -	kmalloc(part->header.NumTransferUnits * sizeof(struct xfer_info_t),
> -		GFP_KERNEL);
> +	part->XferInfo = kmalloc_array(part->header.NumTransferUnits,
> +				       sizeof(*part->XferInfo), GFP_KERNEL);

Another indentation issue is introduced here too...

>      if (!part->XferInfo)
>  	    goto out_EUNInfo;
>  
> @@ -275,8 +274,8 @@ static int build_maps(partition_t *part)
>      memset(part->VirtualBlockMap, 0xff, blocks * sizeof(uint32_t));
>      part->BlocksPerUnit = (1 << header.EraseUnitSize) >> header.BlockSize;
>  
> -    part->bam_cache = kmalloc(part->BlocksPerUnit * sizeof(uint32_t),
> -			      GFP_KERNEL);
> +	part->bam_cache = kmalloc_array(part->BlocksPerUnit,
> +					sizeof(*part->bam_cache), GFP_KERNEL);

+1
>      if (!part->bam_cache)
>  	    goto out_VirtualBlockMap;
>  
>

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

#1557612 — Re: mtd/ftl: Use kmalloc_array() in build_maps()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-01-12 18:00 +0100
SubjectRe: mtd/ftl: Use kmalloc_array() in build_maps()
Message-ID<sYQTf-6jV-11@gated-at.bofh.it>
In reply to#1557426
> The indentation has been changed and the new one looks wrong...

The source code formatting contained various open issues before already.


> If you want to fix the indentation to make it compliant with the Linux
> coding style, do it on the whole file so every thing is uniform.

Such a development task is too much for me today.


> Reviewing such dummy/automatic patches is a pure waste of time,
> so personally I think we should just ignore them.

How much do you care for the usage of a function like “kmalloc_array”?

Regards,
Markus

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

#1557616 — Re: mtd/ftl: Use kmalloc_array() in build_maps()

FromMarek Vasut <marek.vasut@gmail.com>
Date2017-01-12 18:00 +0100
SubjectRe: mtd/ftl: Use kmalloc_array() in build_maps()
Message-ID<sYQTg-6jV-27@gated-at.bofh.it>
In reply to#1557612
On 01/12/2017 05:50 PM, SF Markus Elfring wrote:
>> The indentation has been changed and the new one looks wrong...
> 
> The source code formatting contained various open issues before already.
> 
> 
>> If you want to fix the indentation to make it compliant with the Linux
>> coding style, do it on the whole file so every thing is uniform.
> 
> Such a development task is too much for me today.

Let me officially NAK this patch.

>> Reviewing such dummy/automatic patches is a pure waste of time,
>> so personally I think we should just ignore them.
> 
> How much do you care for the usage of a function like “kmalloc_array”?

Not at all, there are more pressing issues.

-- 
Best regards,
Marek Vasut

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

#1557332 — [PATCH 2/3] mtd/ftl: Delete an error message for a failed memory allocation in ftl_add_mtd()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-01-12 11:40 +0100
Subject[PATCH 2/3] mtd/ftl: Delete an error message for a failed memory allocation in ftl_add_mtd()
Message-ID<sYKXx-2QD-67@gated-at.bofh.it>
In reply to#1557318
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 12 Jan 2017 11:11:47 +0100

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

WARNING: Possible unnecessary 'out of memory' message

Thus fix the affected source code place.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/ftl.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
index ef2f38b6a837..7af23110be6e 100644
--- a/drivers/mtd/ftl.c
+++ b/drivers/mtd/ftl.c
@@ -1042,12 +1042,8 @@ static void ftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
 	partition_t *partition;
 
 	partition = kzalloc(sizeof(partition_t), GFP_KERNEL);
-
-	if (!partition) {
-		printk(KERN_WARNING "No memory to scan for FTL on %s\n",
-		       mtd->name);
+	if (!partition)
 		return;
-	}
 
 	partition->mbd.mtd = mtd;
 
-- 
2.11.0

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

#1557344 — [PATCH 3/3] mtd/ftl: Improve another size determination in ftl_add_mtd()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-01-12 11:50 +0100
Subject[PATCH 3/3] mtd/ftl: Improve another size determination in ftl_add_mtd()
Message-ID<sYL7c-2TT-21@gated-at.bofh.it>
In reply to#1557318
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 12 Jan 2017 11:18:59 +0100

Replace the specification of a data type 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/mtd/ftl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
index 7af23110be6e..70d583aa44ad 100644
--- a/drivers/mtd/ftl.c
+++ b/drivers/mtd/ftl.c
@@ -1041,7 +1041,7 @@ static void ftl_add_mtd(struct mtd_blktrans_ops *tr, struct mtd_info *mtd)
 {
 	partition_t *partition;
 
-	partition = kzalloc(sizeof(partition_t), GFP_KERNEL);
+	partition = kzalloc(sizeof(*partition), GFP_KERNEL);
 	if (!partition)
 		return;
 
-- 
2.11.0

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web