diff mbox

fdt_ro.c: implement strnlen

Message ID 20171018223116.6035-1-programmingkidx@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Programmingkid Oct. 18, 2017, 10:31 p.m. UTC
Implement the strnlen() function if it isn't implemented.

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
---
 libfdt/fdt_ro.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

David Gibson Oct. 19, 2017, 1:31 a.m. UTC | #1
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 <programmingkidx@gmail.com>

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.

> +{
> +    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.

If your environment does not provide strnlen(), you probably need your
own version of libfdt_env.h, which will need to define it.
Programmingkid Oct. 19, 2017, 2:39 a.m. UTC | #2
> On Oct 18, 2017, at 9:31 PM, David Gibson <david@gibson.dropbear.id.au> 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 <programmingkidx@gmail.com>
> 
> 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. 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
David Gibson Oct. 19, 2017, 4:11 a.m. UTC | #3
On Wed, Oct 18, 2017 at 10:39:57PM -0400, Programmingkid wrote:
> 
> > On Oct 18, 2017, at 9:31 PM, David Gibson <david@gibson.dropbear.id.au> 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 <programmingkidx@gmail.com>
> > 
> > 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).
Peter Maydell Oct. 19, 2017, 9:37 a.m. UTC | #4
On 19 October 2017 at 05:11, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Wed, Oct 18, 2017 at 10:39:57PM -0400, Programmingkid wrote:
>> 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.

It's pretty poor, but the 10.7 release date was 2011, five years
ago now, and it's been out of security-fix support for 2 or 3 years.

thanks
-- PMM
Programmingkid Oct. 19, 2017, 2:54 p.m. UTC | #5
> On Oct 19, 2017, at 12:11 AM, David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> On Wed, Oct 18, 2017 at 10:39:57PM -0400, Programmingkid wrote:
>> 
>>> On Oct 18, 2017, at 9:31 PM, David Gibson <david@gibson.dropbear.id.au> 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 <programmingkidx@gmail.com>
>>> 
>>> 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?
diff mbox

Patch

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 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)
+{
+    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)
 {