[PATCH] nfc: hci: Save a few bytes of memory when registering a 'nfc_llc' engine

[PATCH] nfc: hci: Save a few bytes of memory when registering a 'nfc_llc' engine

[Christophe JAILLET]

nfc_llc_register() calls pass a string literal as the 'name' parameter.

So kstrdup_const() can be used instead of kfree() to avoid a memory
allocation in such cases.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 net/nfc/hci/llc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/hci/llc.c b/net/nfc/hci/llc.c
index 2140f6724644..8c7b5a817b25 100644
--- a/net/nfc/hci/llc.c
+++ b/net/nfc/hci/llc.c
@@ -49,7 +49,7 @@ int nfc_llc_register(const char *name, const struct nfc_llc_ops *ops)
 	if (llc_engine == NULL)
 		return -ENOMEM;
 
-	llc_engine->name = kstrdup(name, GFP_KERNEL);
+	llc_engine->name = kstrdup_const(name, GFP_KERNEL);
 	if (llc_engine->name == NULL) {
 		kfree(llc_engine);
 		return -ENOMEM;
@@ -83,7 +83,7 @@ void nfc_llc_unregister(const char *name)
 		return;
 
 	list_del(&llc_engine->entry);
-	kfree(llc_engine->name);
+	kfree_const(llc_engine->name);
 	kfree(llc_engine);
 }
 
-- 
2.43.0

Re: [PATCH] nfc: hci: Save a few bytes of memory when registering a 'nfc_llc' engine

[Simon Horman]

On Sat, Jan 20, 2024 at 10:56:06AM +0100, Christophe JAILLET wrote:
> nfc_llc_register() calls pass a string literal as the 'name' parameter.
> 
> So kstrdup_const() can be used instead of kfree() to avoid a memory
> allocation in such cases.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

## Form letter - net-next-closed

[adapted from text by Jakub]

The merge window for v6.8 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.

Please repost when net-next reopens on or after 22nd January.

RFC patches sent for review only are obviously welcome at any time.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
--
pw-bot: defer

Re: [PATCH] nfc: hci: Save a few bytes of memory when registering a 'nfc_llc' engine

[Krzysztof Kozlowski]

On 20/01/2024 10:56, Christophe JAILLET wrote:
> nfc_llc_register() calls pass a string literal as the 'name' parameter.
> 
> So kstrdup_const() can be used instead of kfree() to avoid a memory
> allocation in such cases.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

You probably need to resend it, because that time net-next was closed.
If resending, keep my tag.

Best regards,
Krzysztof

[PATCH RESEND] nfc: hci: Save a few bytes of memory when registering a 'nfc_llc' engine

[Christophe JAILLET]

nfc_llc_register() calls pass a string literal as the 'name' parameter.

So kstrdup_const() can be used instead of kfree() to avoid a memory
allocation in such cases.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
Resent because of 'net-next-closed' message on previous try.

Added R-b tag. (see [1])

[1]: https://lore.kernel.org/all/9415e571-50a1-41d3-8205-68e4128bbe6d@linaro.org/
---
 net/nfc/hci/llc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/hci/llc.c b/net/nfc/hci/llc.c
index 2140f6724644..8c7b5a817b25 100644
--- a/net/nfc/hci/llc.c
+++ b/net/nfc/hci/llc.c
@@ -49,7 +49,7 @@ int nfc_llc_register(const char *name, const struct nfc_llc_ops *ops)
 	if (llc_engine == NULL)
 		return -ENOMEM;
 
-	llc_engine->name = kstrdup(name, GFP_KERNEL);
+	llc_engine->name = kstrdup_const(name, GFP_KERNEL);
 	if (llc_engine->name == NULL) {
 		kfree(llc_engine);
 		return -ENOMEM;
@@ -83,7 +83,7 @@ void nfc_llc_unregister(const char *name)
 		return;
 
 	list_del(&llc_engine->entry);
-	kfree(llc_engine->name);
+	kfree_const(llc_engine->name);
 	kfree(llc_engine);
 }
 
-- 
2.43.0

Re: [PATCH RESEND] nfc: hci: Save a few bytes of memory when registering a 'nfc_llc' engine

[Jakub Kicinski]

On Sat, 27 Jan 2024 10:58:29 +0100 Christophe JAILLET wrote:
> nfc_llc_register() calls pass a string literal as the 'name' parameter.
> 
> So kstrdup_const() can be used instead of kfree() to avoid a memory
> allocation in such cases.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

There is a kfree() call in nfc_llc_exit() that looks suspiciously
like it may also free the name.

Re: [PATCH RESEND] nfc: hci: Save a few bytes of memory when registering a 'nfc_llc' engine

[Christophe JAILLET]

Le 01/02/2024 à 00:08, Jakub Kicinski a écrit :
> On Sat, 27 Jan 2024 10:58:29 +0100 Christophe JAILLET wrote:
>> nfc_llc_register() calls pass a string literal as the 'name' parameter.
>>
>> So kstrdup_const() can be used instead of kfree() to avoid a memory
>> allocation in such cases.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> There is a kfree() call in nfc_llc_exit() that looks suspiciously
> like it may also free the name.
> 
> 

Hi,

you are right.


Should we have something like:

void nfc_llc_exit(void)
{
	struct nfc_llc_engine *llc_engine, *n;

	list_for_each_entry_safe(llc_engine, n, &llc_engines, entry)
		nfc_llc_unregister(&llc_engine->name);
}


It would be slower, but it would reduce code duplication as well.
This is just an _exit() function, so it shouldn't be called that often 
anyway, if called at all.

Or, add another function with the list_del()+kfree_const()+kfree(), that 
would be called from nfc_llc_exit() and nfc_llc_unregister(), to have 
the best of the 2 worlds?

CJ

Re: [PATCH RESEND] nfc: hci: Save a few bytes of memory when registering a 'nfc_llc' engine

[Jakub Kicinski]

On Fri, 2 Feb 2024 20:11:56 +0100 Christophe JAILLET wrote:
> It would be slower, but it would reduce code duplication as well.
> This is just an _exit() function, so it shouldn't be called that often 
> anyway, if called at all.
> 
> Or, add another function with the list_del()+kfree_const()+kfree(), that 
> would be called from nfc_llc_exit() and nfc_llc_unregister(), to have 
> the best of the 2 worlds?

My vote is the latter - factor out the 3 calls into a new helper, call
it where appropriate.