On 2025/1/11 17:17, Krzysztof Kozlowski wrote:
> On Sat, Jan 11, 2025 at 07:48:48AM +0800, Zijun Hu wrote:
>> On 2025/1/11 04:41, Rob Herring wrote:
>>> On Thu, Jan 09, 2025 at 09:27:05PM +0800, Zijun Hu wrote:
>>>> From: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
>>>>
>>>> __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.
> 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)
{
unsigned char c1, c2;
if (!cs || !ct)
return false;
do {
c1 = *cs++;
c2 = *ct++;
if (c1 != c2)
return c2 == '\0';
} while (c1 != '\0');
return true;
}
>
> Best regards,
> Krzysztof
>
