* [PATCH 0/3] MTD-FTL: Fine-tuning for two function implementations
@ 2017-01-12 10:33 SF Markus Elfring
2017-01-12 10:35 ` [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps() SF Markus Elfring
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: SF Markus Elfring @ 2017-01-12 10:33 UTC (permalink / raw)
To: linux-mtd, Boris Brezillon, Brian Norris, Cyrille Pitchen,
David Woodhouse, Marek Vasut, Richard Weinberger
Cc: LKML, kernel-janitors
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps()
2017-01-12 10:33 [PATCH 0/3] MTD-FTL: Fine-tuning for two function implementations SF Markus Elfring
@ 2017-01-12 10:35 ` SF Markus Elfring
2017-01-12 13:00 ` Cyrille Pitchen
2017-01-13 9:32 ` [PATCH 1/3] " kbuild test robot
2017-01-12 10:36 ` [PATCH 2/3] mtd/ftl: Delete an error message for a failed memory allocation in ftl_add_mtd() SF Markus Elfring
2017-01-12 10:38 ` [PATCH 3/3] mtd/ftl: Improve another size determination " SF Markus Elfring
2 siblings, 2 replies; 8+ messages in thread
From: SF Markus Elfring @ 2017-01-12 10:35 UTC (permalink / raw)
To: linux-mtd, Boris Brezillon, Brian Norris, Cyrille Pitchen,
David Woodhouse, Marek Vasut, Richard Weinberger
Cc: LKML, kernel-janitors
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] mtd/ftl: Delete an error message for a failed memory allocation in ftl_add_mtd()
2017-01-12 10:33 [PATCH 0/3] MTD-FTL: Fine-tuning for two function implementations SF Markus Elfring
2017-01-12 10:35 ` [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps() SF Markus Elfring
@ 2017-01-12 10:36 ` SF Markus Elfring
2017-01-12 10:38 ` [PATCH 3/3] mtd/ftl: Improve another size determination " SF Markus Elfring
2 siblings, 0 replies; 8+ messages in thread
From: SF Markus Elfring @ 2017-01-12 10:36 UTC (permalink / raw)
To: linux-mtd, Boris Brezillon, Brian Norris, Cyrille Pitchen,
David Woodhouse, Marek Vasut, Richard Weinberger, Wolfram Sang
Cc: LKML, kernel-janitors
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] mtd/ftl: Improve another size determination in ftl_add_mtd()
2017-01-12 10:33 [PATCH 0/3] MTD-FTL: Fine-tuning for two function implementations SF Markus Elfring
2017-01-12 10:35 ` [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps() SF Markus Elfring
2017-01-12 10:36 ` [PATCH 2/3] mtd/ftl: Delete an error message for a failed memory allocation in ftl_add_mtd() SF Markus Elfring
@ 2017-01-12 10:38 ` SF Markus Elfring
2 siblings, 0 replies; 8+ messages in thread
From: SF Markus Elfring @ 2017-01-12 10:38 UTC (permalink / raw)
To: linux-mtd, Boris Brezillon, Brian Norris, Cyrille Pitchen,
David Woodhouse, Marek Vasut, Richard Weinberger
Cc: LKML, kernel-janitors
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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps()
2017-01-12 10:35 ` [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps() SF Markus Elfring
@ 2017-01-12 13:00 ` Cyrille Pitchen
2017-01-12 16:50 ` SF Markus Elfring
2017-01-13 9:32 ` [PATCH 1/3] " kbuild test robot
1 sibling, 1 reply; 8+ messages in thread
From: Cyrille Pitchen @ 2017-01-12 13:00 UTC (permalink / raw)
To: SF Markus Elfring, linux-mtd, Boris Brezillon, Brian Norris,
David Woodhouse, Marek Vasut, Richard Weinberger
Cc: LKML, kernel-janitors
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;
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mtd/ftl: Use kmalloc_array() in build_maps()
2017-01-12 13:00 ` Cyrille Pitchen
@ 2017-01-12 16:50 ` SF Markus Elfring
2017-01-12 16:58 ` Marek Vasut
0 siblings, 1 reply; 8+ messages in thread
From: SF Markus Elfring @ 2017-01-12 16:50 UTC (permalink / raw)
To: Cyrille Pitchen, linux-mtd
Cc: Boris Brezillon, Brian Norris, David Woodhouse, Marek Vasut,
Richard Weinberger, LKML, kernel-janitors
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: mtd/ftl: Use kmalloc_array() in build_maps()
2017-01-12 16:50 ` SF Markus Elfring
@ 2017-01-12 16:58 ` Marek Vasut
0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2017-01-12 16:58 UTC (permalink / raw)
To: SF Markus Elfring, Cyrille Pitchen, linux-mtd
Cc: Boris Brezillon, Richard Weinberger, kernel-janitors, LKML,
Brian Norris, David Woodhouse
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps()
2017-01-12 10:35 ` [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps() SF Markus Elfring
2017-01-12 13:00 ` Cyrille Pitchen
@ 2017-01-13 9:32 ` kbuild test robot
1 sibling, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2017-01-13 9:32 UTC (permalink / raw)
To: SF Markus Elfring
Cc: kbuild-all, linux-mtd, Boris Brezillon, Brian Norris,
Cyrille Pitchen, David Woodhouse, Marek Vasut,
Richard Weinberger, LKML, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 3485 bytes --]
Hi Markus,
[auto build test WARNING on mtd/master]
[also build test WARNING on v4.10-rc3 next-20170113]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/SF-Markus-Elfring/MTD-FTL-Fine-tuning-for-two-function-implementations/20170113-163618
base: git://git.infradead.org/linux-mtd.git master
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All warnings (new ones prefixed by >>):
drivers/mtd/ftl.c: In function 'build_maps':
>> drivers/mtd/ftl.c:214:5: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
for (i = 0; i < part->DataUnits; i++)
^~~
drivers/mtd/ftl.c:216:2: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'for'
part->XferInfo = kmalloc_array(part->header.NumTransferUnits,
^~~~
vim +/for +214 drivers/mtd/ftl.c
^1da177e Linus Torvalds 2005-04-16 198 static int build_maps(partition_t *part)
^1da177e Linus Torvalds 2005-04-16 199 {
^1da177e Linus Torvalds 2005-04-16 200 erase_unit_header_t header;
3854be77 David Woodhouse 2008-12-10 201 uint16_t xvalid, xtrans, i;
3854be77 David Woodhouse 2008-12-10 202 unsigned blocks, j;
^1da177e Linus Torvalds 2005-04-16 203 int hdr_ok, ret = -1;
^1da177e Linus Torvalds 2005-04-16 204 ssize_t retval;
^1da177e Linus Torvalds 2005-04-16 205 loff_t offset;
^1da177e Linus Torvalds 2005-04-16 206
^1da177e Linus Torvalds 2005-04-16 207 /* Set up erase unit maps */
^1da177e Linus Torvalds 2005-04-16 208 part->DataUnits = le16_to_cpu(part->header.NumEraseUnits) -
^1da177e Linus Torvalds 2005-04-16 209 part->header.NumTransferUnits;
b6c411b0 Markus Elfring 2017-01-12 210 part->EUNInfo = kmalloc_array(part->DataUnits, sizeof(*part->EUNInfo),
^1da177e Linus Torvalds 2005-04-16 211 GFP_KERNEL);
^1da177e Linus Torvalds 2005-04-16 212 if (!part->EUNInfo)
^1da177e Linus Torvalds 2005-04-16 213 goto out;
^1da177e Linus Torvalds 2005-04-16 @214 for (i = 0; i < part->DataUnits; i++)
^1da177e Linus Torvalds 2005-04-16 215 part->EUNInfo[i].Offset = 0xffffffff;
b6c411b0 Markus Elfring 2017-01-12 216 part->XferInfo = kmalloc_array(part->header.NumTransferUnits,
b6c411b0 Markus Elfring 2017-01-12 217 sizeof(*part->XferInfo), GFP_KERNEL);
^1da177e Linus Torvalds 2005-04-16 218 if (!part->XferInfo)
^1da177e Linus Torvalds 2005-04-16 219 goto out_EUNInfo;
^1da177e Linus Torvalds 2005-04-16 220
^1da177e Linus Torvalds 2005-04-16 221 xvalid = xtrans = 0;
^1da177e Linus Torvalds 2005-04-16 222 for (i = 0; i < le16_to_cpu(part->header.NumEraseUnits); i++) {
:::::: The code at line 214 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 48625 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-01-13 9:32 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-12 10:33 [PATCH 0/3] MTD-FTL: Fine-tuning for two function implementations SF Markus Elfring
2017-01-12 10:35 ` [PATCH 1/3] mtd/ftl: Use kmalloc_array() in build_maps() SF Markus Elfring
2017-01-12 13:00 ` Cyrille Pitchen
2017-01-12 16:50 ` SF Markus Elfring
2017-01-12 16:58 ` Marek Vasut
2017-01-13 9:32 ` [PATCH 1/3] " kbuild test robot
2017-01-12 10:36 ` [PATCH 2/3] mtd/ftl: Delete an error message for a failed memory allocation in ftl_add_mtd() SF Markus Elfring
2017-01-12 10:38 ` [PATCH 3/3] mtd/ftl: Improve another size determination " SF Markus Elfring
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).