Re: [Qemu-devel] [PATCH] fdt_ro.c: implement strnlen

[Programmingkid]

> On Oct 19, 2017, at 12:11 AM, David Gibson <address@hidden> wrote:
> 
> On Wed, Oct 18, 2017 at 10:39:57PM -0400, Programmingkid wrote:
>> 
>>> On Oct 18, 2017, at 9:31 PM, David Gibson <address@hidden> wrote:
>>> 
>>> On Wed, Oct 18, 2017 at 06:31:16PM -0400, John Arbuckle wrote:
>>>> Implement the strnlen() function if it isn't implemented.
>>>> 
>>>> Signed-off-by: John Arbuckle <address@hidden>
>>> 
>>> Nice idea, but this won't work.
>>> 
>>>> ---
>>>> libfdt/fdt_ro.c | 24 ++++++++++++++++++++++++
>>>> 1 file changed, 24 insertions(+)
>>>> 
>>>> diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
>>>> index 3d00d2e..a7986fb 100644
>>>> --- a/libfdt/fdt_ro.c
>>>> +++ b/libfdt/fdt_ro.c
>>>> @@ -55,6 +55,30 @@
>>>> 
>>>> #include "libfdt_internal.h"
>>>> 
>>>> +/* if the current environment does not define strnlen */
>>>> +#ifndef strnlen
>>> 
>>> This will only trigger if strnlen is defined as a macro.  The C
>>> library (or other environment) *might* do that, but there's no
>>> guarantee, whether or not it defines strnlen as a function.  With
>>> extra complications if the compiler has a strnlen builtin like gcc.
>> 
>> :(
>> 
>>>> +
>>>> +/* This eliminates the missing prototype warning */
>>>> +int strnlen(const char *string, int max_count);
>>>> +
>>>> +/*
>>>> + * strnlen: return the length of a string or max_count
>>>> + * which ever is shortest
>>>> + */
>>>> +
>>>> +int strnlen(const char *string, int max_count)
>>> 
>>> Also strnlen is supposed to take and return size_t, not int.
>> 
>> Will change this.
>> 
>>> 
>>>> +{
>>>> +    int count;
>>>> +    for(count = 0; count < max_count; count++) {
>>>> +        if (string[count] == '\0') {
>>>> +            break;
>>>> +        }
>>>> +    }
>>>> +    return count;
>>>> +}
>>>> +
>>>> +#endif /* strnlen */
>>>> +
>>>> static int _fdt_nodename_eq(const void *fdt, int offset,
>>>>                        const char *s, int len)
>>>> {
>>> 
>>> In any case, this sort of compatibility munging is the job of
>>> libfdt_env.h.  It's purpose is to provide all the external things that
>>> libfdt needs - there's not that many of them and strnlen() is one.
>>> The included libfdt_env.h is intended for POSIXy userspace
>>> environments, which should already include strnlen() in string.h.
>> 
>> QEMU supports Mac OS 10.5 and higher. The strnlen() function might
>> have been added to Mac OS X in version 10.7.
> 
> Wow.  No strnlen() in MacOS until that recently, that's pretty shit.
> 
>> libfdt prevents QEMU
>> from compiling on Mac OS X because of the missing strnlen()
>> function. This issue needs to be resolved. If libfdt_env.h is where
>> you want a fix located, would something like this be good:
> 
>> 
>> #ifdef __APPLE__
>> 
>> #if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
>>      size_t strnlen(const char *string, int max_count)
>>      {
>>              ... // pretend this is implemented
>>      }
>> #endif 
>> 
>> #endif
> 
> I'm not sure I'd go as far as to call it "good" - it's kinda ugly -
> but it would be an acceptable solution (as long as max_count is
> changed to size_t as well, of course).

Yes, you are right. It is an eye sore. Libfdt is part of the Device Tree 
Compiler toolchain. This toolchain is something that can exist independently of 
QEMU. Someone could download the Device Tree Compiler toolchain and try to 
build it on Mac OS 10.5. This project doesn't have a configuration script but 
it does have a makefile. What I am thinking is add a target to the makefile 
that adds the strnlen() function to the toolchain if the user is building on 
anything less than Mac OS 10.7. The Makefile.libfdt file in the libfdt folder 
looks like a good place to put this code. This might be better looking. 
Thoughts?