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

[Alexander Graf]

On 26.08.18 21:36, Heinrich Schuchardt wrote:
> 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.

I think we are in agreement, I just wanted to point out that u"text"
creates a utf-16 string rather than a utf-8 one :).

Unfortunately I don't know when we can bump the compiler requirement,
but it can't be forever and we should keep it in mind, I agree.

> 
>>
>>>
>>> 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?

I seem to recall that gcc at least allowed for strcpy() and friends to
get inlined. The point was really more about using function names that
do exactly what people are used to.

I don't have a terribly string feeling towards it though, as strlen() is
definitely much wider used than wcslen() and thus much more well known.


Alex