Thread (1333 messages) 1333 messages, 109 authors, 2024-01-05

Re: [PATCH 01/10] firewire-net: Use kmalloc_array() in fwnet_broadcast_start()

From: SF Markus Elfring <hidden>
Date: 2016-09-24 15:29:35
Also in: lkml

quoted
@@ -1103,8 +1103,7 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
 
 	max_receive = 1U << (dev->card->max_receive + 1);
 	num_packets = (FWNET_ISO_PAGE_COUNT * PAGE_SIZE) / max_receive;
-
-	ptrptr = kmalloc(sizeof(void *) * num_packets, GFP_KERNEL);
+	ptrptr = kmalloc_array(num_packets, sizeof(*ptrptr), GFP_KERNEL);
 	if (!ptrptr) {
 		retval = -ENOMEM;
 		goto failed;
Coccinelle enabled you to determine that kmalloc_array /could/ be used here.
A script for the semantic patch language pointed hundreds of source files out
with such software update opportunities.

But whether it /should/ be used here is another question, and it is
not addressed in your changelog.
I can expand the corresponding description when it will be desired.

(You state that there is an "issue" but do not explain.)
Do you prefer an other wording for such an update candidate?

kmalloc_array is a kmalloc wrapper which adds an inline check for integer
overflow.  So, can sizeof(void *) * num_packets ever overflow size_t?

If yes,
Is there a probability that the calculated number of packets will become
too big for the preferred system limits anyhow?

	do we want a runtime check here (which kmalloc_array provides),
Did you notice the information from the commit "mm: faster kmalloc_array(), kcalloc()"
(from 2016-07-26) already?
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id‘c6a05f72a996bee5133e76374ab3ad7d3b9b72

	or do we want a compile-time check?
I guess that some software developers and subsystem maintainers are looking
for a bit more clarification around involved design dependencies.

If no,
	then the remaining benefit of the patch is that it is more obvious
	to the reader that dev->broadcast_rcv_buffer_ptrs is an array,
How do you value such a kind of source code annotation?

	but possibly at the cost of superfluous code.
How do you think about to care for a bit more consistent use of Linux programming interfaces?

	Is gcc's optimizer able to resolve kmalloc_array's check at compile time
	as always false, such that the superfluous code is eliminated as dead code?
Which versions of compiler implementations would you like to check further?

I believe I know answers to this but prefer to hear what you as the patch
author think about it.
I presented another update suggestion also for this software module as a result
from a general source code search pattern.
The corresponding change acceptance varies and is evolving as usual.

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help