[U-Boot] [PATCH 02/15] efi_loader: rename utf16_strlen, utf16_strnlen

[Heinrich Schuchardt]

On 08/26/2018 08:33 PM, Alexander Graf wrote:
> 
> 
> On 26.08.18 20:21, Heinrich Schuchardt wrote:
>> On 08/26/2018 07:52 PM, Alexander Graf wrote:
>>>
>>>
>>> On 11.08.18 17:28, Heinrich Schuchardt wrote:
>>>> The function names utf16_strlen() and utf16_strnlen() are misnomers.
>>>> The functions do not count utf-16 characters but non-zero words.
>>>> So let's rename them to u16_strlen and u16_strnlen().
>>>>
>>>> In utf16_dup() avoid assignment in if clause.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>> ---
>>>>  include/charset.h             | 28 +++++++++++-----------------
>>>>  lib/charset.c                 | 10 +++++++---
>>>>  lib/efi_loader/efi_bootmgr.c  |  2 +-
>>>>  lib/efi_loader/efi_console.c  |  2 +-
>>>>  lib/efi_loader/efi_file.c     |  2 +-
>>>>  lib/efi_loader/efi_variable.c |  2 +-
>>>>  lib/vsprintf.c                |  2 +-
>>>>  7 files changed, 23 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/include/charset.h b/include/charset.h
>>>> index 11832cbd12..2307559890 100644
>>>> --- a/include/charset.h
>>>> +++ b/include/charset.h
>>>> @@ -13,29 +13,23 @@
>>>>  #define MAX_UTF8_PER_UTF16 3
>>>>  
>>>>  /**
>>>> - * utf16_strlen() - Get the length of an utf16 string
>>>> + * u16_strlen - count non-zero words
>>>
>>> This really just implements wcslen() now, right?
>>>
>>>>   *
>>>> - * Returns the number of 16 bit characters in an utf16 string, not
>>>> - * including the terminating NULL character.
>>>> - *
>>>> - * @in     the string to measure
>>>> - * @return the string length
>>>> + * @in:			utf-16 string
>>>
>>> Is "in" really a utf-16 string? Probably rather a null-terminated string
>>> of words.
>>
>> I will update the comment.
>>
>>>
>>>> + * ReturnValue:		number of non-zero words.
>>>> + *			This is not the number of utf-16 letters!
>>>>   */
>>>> -size_t utf16_strlen(const uint16_t *in);
>>>> +size_t u16_strlen(const u16 *in);
>>>>  
>>>>  /**
>>>> - * utf16_strnlen() - Get the length of a fixed-size utf16 string.
>>>> - *
>>>> - * Returns the number of 16 bit characters in an utf16 string,
>>>> - * not including the terminating NULL character, but at most
>>>> - * 'count' number of characters.  In doing this, utf16_strnlen()
>>>> - * looks at only the first 'count' characters.
>>>> + * u16_strlen - count non-zero words
>>>
>>> This really just implements wcsnlen() now, right?
>>
>> Currently we have set wchar size to 16bit using a compiler flag. In my
>> opinion this was not necessary. In C11 we could have use the u"text"
>> notation for utf-8 string constants instead of L"text".
> 
> I thought the idea was to get utf-16 string constants?

Yes, but the C11 way is not using a compiler flag but using the right
prefix (which was not available with C99). So once we do not have to
support outdated gcc versions anymore we could get rid of the compiler flag.

> 
>>
>> This function really is for u16[] and not for wchar_t[].
>>
>> I would hesitate to call this function wcsnlen() as the working of
>> wcsnlen() depends on said compiler setting.
> 
> *shrug* either way works for me. By calling them their official names we
> could've potentially given gcc the chance to optimize/inline them better.

Do have any hint that gcc really provides better compilation results
based on the function name?

>From what I see in the gcc code only the address sanitizer is aware that
wcsnlen() is accessing a memory range.

Best regards

Heinrich

> 
> 
> Alex
>