Thread (56 messages) 56 messages, 5 authors, 2025-02-27

Re: [PATCH v4 14/14] of: Improve __of_add_property_sysfs() readability

From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2025-01-13 08:33:36
Also in: lkml

On Sat, Jan 11, 2025 at 08:44:34PM +0800, Zijun Hu wrote:
On 2025/1/11 17:17, Krzysztof Kozlowski wrote:
quoted
On Sat, Jan 11, 2025 at 07:48:48AM +0800, Zijun Hu wrote:
quoted
On 2025/1/11 04:41, Rob Herring wrote:
quoted
On Thu, Jan 09, 2025 at 09:27:05PM +0800, Zijun Hu wrote:
quoted
From: Zijun Hu <redacted>

__of_add_property_sysfs() hard codes string "security-" length as 9, but
that is not obvious for readers.

Improve its readability by using strlen().
Does the compiler optimize the strlen call away? Maybe, maybe not. If 
not, that's N calls to strlen() where N is the number of properties in 
your DT. That's in the 1000s easily.

Do you really want to go test enough compiler versions we support to 
feel confident this is optimized away. I don't.
i understand your concern about performance.
perhaps, such impact about performance may be ignored for linux OS.

what about below solution ?

const char security_prefix[] = "security-";
use 'sizeof(security_prefix) - 1' for the length of string.
Code is still not equivalent - just de-assemble it and you will see
some overhead.
Thank you Krzysztof Kozlowski for code review

the overhead may be that use of stack is increased a bit.
it may not almost impact performance since 'sizeof(security_prefix) - 1'
may be still a compile time constant.
No, I was talking about initialization. Look how it is done.
quoted
Maybe just introduce builtin_strlen() to string.h and use such? It would
be the pretty obvious code.
it is a good idea to introduce API for this common issue, it seems the
main strncmp() usages in current kernel tree is to check if a string
begin with the other string a shown below:

// "s2_string" is wrote twice, and also impact performance
strncmp(s1, "s2_string", strlen("s2_string"))

// it is not obvious for readers that "s2_string" length is 9
strncmp(s1, "s2_string", 9)

//"s2_string" is wrote twice, same as strcmp(), may be wrong
strncmp(s1, "s2_string", sizeof("s2_string"))

//"s2_string" is wrote twice
strncmp(s1, "s2_string", sizeof("s2_string") - 1)

what about a new API such as ?

// check if @s1 begins with @s2
bool str_begin_with(const char *cs, const char *ct)
Nothing related to my proposal, so no clue why you ask me. Anyway, as it
is one more unsafe str-like function, I don't see how it makes things
much better.

Best regards,
Krzysztof
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help