[Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[John Arbuckle]

Prior the Mac OS 10.7, the function strnlen() was not available. This patch
implements strnlen() on Mac OS X versions that are below 10.7.

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
---
v2 changes:
- Simplified the code to make it static inline'ed
- Changed the type of count to size_t

 libfdt/libfdt_env.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
index 952056c..2569339 100644
--- a/libfdt/libfdt_env.h
+++ b/libfdt/libfdt_env.h
@@ -109,4 +109,33 @@ static inline fdt64_t cpu_to_fdt64(uint64_t x)
 #undef CPU_TO_FDT16
 #undef EXTRACT_BYTE

+#ifdef __APPLE__
+#include <AvailabilityMacros.h>
+
+#define MAC_OS_X_VERSION_10_7 1070
+
+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
+
+/*
+ * strnlen: returns the length of a string or max_count - which ever is smallest
+ * Input 1 string: the string whose size is to be determined
+ * Input 2 max_count: the maximum value returned by this function
+ * Output: length of the string or max_count (the smallest of the two)
+ */
+static inline size_t strnlen(const char *string, size_t max_count)
+{
+    size_t count;
+    for (count = 0; count < max_count; count++) {
+        if (string[count] == '\0') {
+            break;
+        }
+    }
+    return count;
+}
+
+#endif /* (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7) */
+
+#endif /* __APPLE__ */
+
 #endif /* _LIBFDT_ENV_H */
-- 
2.13.5 (Apple Git-94)

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[Richard Henderson]

On 10/20/2017 10:55 AM, John Arbuckle wrote:
> +static inline size_t strnlen(const char *string, size_t max_count)
> +{
> +    size_t count;
> +    for (count = 0; count < max_count; count++) {
> +        if (string[count] == '\0') {
> +            break;
> +        }
> +    }
> +    return count;

Not to nitpick, but

  const char *p = memchr(string, 0, max_count);
  return p ? max_count : p - string;


r~

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
> On 10/20/2017 10:55 AM, John Arbuckle wrote:
> > +static inline size_t strnlen(const char *string, size_t max_count)
> > +{
> > +    size_t count;
> > +    for (count = 0; count < max_count; count++) {
> > +        if (string[count] == '\0') {
> > +            break;
> > +        }
> > +    }
> > +    return count;
> 
> Not to nitpick, but
> 
>   const char *p = memchr(string, 0, max_count);
>   return p ? max_count : p - string;

Richard's right, that's definitely a better implementation.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[Programmingkid]

> On Oct 22, 2017, at 1:33 AM, David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
>> On 10/20/2017 10:55 AM, John Arbuckle wrote:
>>> +static inline size_t strnlen(const char *string, size_t max_count)
>>> +{
>>> +    size_t count;
>>> +    for (count = 0; count < max_count; count++) {
>>> +        if (string[count] == '\0') {
>>> +            break;
>>> +        }
>>> +    }
>>> +    return count;
>> 
>> Not to nitpick, but
>> 
>>  const char *p = memchr(string, 0, max_count);
>>  return p ? max_count : p - string;
> 
> Richard's right, that's definitely a better implementation.

His implementation is smaller, but this one is even smaller. Plus it uses the familiar strlen() function:

size_t strnlen(const char *string, size_t max_count)
{
    return strlen(string) < max_count ? strlen(string) : max_count;
}

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[John Reiser]

> ...  this one is even smaller. Plus it uses the familiar strlen() function:
> 
> size_t strnlen(const char *string, size_t max_count)
> {
>      return strlen(string) < max_count ? strlen(string) : max_count;
> }
> 

Please do not use that implementation.
The major goal of strnlen is to avoid looking beyond &string[max_count].
strlen(string) looks all the way to the end, which may be very much longer than max_count;
and which may cause SIGSEGV by running into a memory page that does not exist
before the terminating '\0' is found.
[Besides, some compilers do not recognize that "strlen(string)"
need not be evaluated twice.]

-- 

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[Ian Lepore]

On Sun, 2017-10-22 at 10:41 -0400, Programmingkid wrote:
> > 
> > On Oct 22, 2017, at 1:33 AM, David Gibson  wrote:
> > 
> > On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
> > > 
> > > On 10/20/2017 10:55 AM, John Arbuckle wrote:
> > > > 
> > > > +static inline size_t strnlen(const char *string, size_t max_count)
> > > > +{
> > > > +    size_t count;
> > > > +    for (count = 0; count < max_count; count++) {
> > > > +        if (string[count] == '\0') {
> > > > +            break;
> > > > +        }
> > > > +    }
> > > > +    return count;
> > > Not to nitpick, but
> > > 
> > >  const char *p = memchr(string, 0, max_count);
> > >  return p ? max_count : p - string;
> > Richard's right, that's definitely a better implementation.
> His implementation is smaller, but this one is even smaller. Plus it uses the familiar strlen() function:
> 
> size_t strnlen(const char *string, size_t max_count)
> {
>     return strlen(string) < max_count ? strlen(string) : max_count;
> }

That is not a proper implementation of strnlen(), which is not supposed
to access any source-string bytes beyond max_count.

-- Ian

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[Programmingkid]

> On Oct 22, 2017, at 3:06 PM, Ian Lepore <ian@freebsd.org> wrote:
> 
> On Sun, 2017-10-22 at 10:41 -0400, Programmingkid wrote:
>>> 
>>> On Oct 22, 2017, at 1:33 AM, David Gibson  wrote:
>>> 
>>> On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
>>>> 
>>>> On 10/20/2017 10:55 AM, John Arbuckle wrote:
>>>>> 
>>>>> +static inline size_t strnlen(const char *string, size_t max_count)
>>>>> +{
>>>>> +    size_t count;
>>>>> +    for (count = 0; count < max_count; count++) {
>>>>> +        if (string[count] == '\0') {
>>>>> +            break;
>>>>> +        }
>>>>> +    }
>>>>> +    return count;
>>>> Not to nitpick, but
>>>> 
>>>>  const char *p = memchr(string, 0, max_count);
>>>>  return p ? max_count : p - string;
>>> Richard's right, that's definitely a better implementation.
>> His implementation is smaller, but this one is even smaller. Plus it uses the familiar strlen() function:
>> 
>> size_t strnlen(const char *string, size_t max_count)
>> {
>>     return strlen(string) < max_count ? strlen(string) : max_count;
>> }
> 
> That is not a proper implementation of strnlen(), which is not supposed
> to access any source-string bytes beyond max_count.
> 
> -- Ian

http://pubs.opengroup.org/onlinepubs/9699919799/functions/strlen.html
This specification document should help anyone who wants more info.

The first implementation using the loop would never access anything beyond max_count. My second implementation does go beyond max_count. The implementation using memchr() will probably live up the requirement so I guess it wins.

Thank you Ian for this information. 

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[Programmingkid]

> On Oct 22, 2017, at 1:33 AM, David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
>> On 10/20/2017 10:55 AM, John Arbuckle wrote:
>>> +static inline size_t strnlen(const char *string, size_t max_count)
>>> +{
>>> +    size_t count;
>>> +    for (count = 0; count < max_count; count++) {
>>> +        if (string[count] == '\0') {
>>> +            break;
>>> +        }
>>> +    }
>>> +    return count;
>> 
>> Not to nitpick, but
>> 
>>  const char *p = memchr(string, 0, max_count);
>>  return p ? max_count : p - string;
> 
> Richard's right, that's definitely a better implementation.

I was just wondering, what if we rewrote the code to use strlen() instead of strnlen(). Would that be an acceptable solution? 

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]

On Tue, Oct 24, 2017 at 12:16:47AM -0400, Programmingkid wrote:
> 
> > On Oct 22, 2017, at 1:33 AM, David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > On Fri, Oct 20, 2017 at 04:44:58PM -0700, Richard Henderson wrote:
> >> On 10/20/2017 10:55 AM, John Arbuckle wrote:
> >>> +static inline size_t strnlen(const char *string, size_t max_count)
> >>> +{
> >>> +    size_t count;
> >>> +    for (count = 0; count < max_count; count++) {
> >>> +        if (string[count] == '\0') {
> >>> +            break;
> >>> +        }
> >>> +    }
> >>> +    return count;
> >> 
> >> Not to nitpick, but
> >> 
> >>  const char *p = memchr(string, 0, max_count);
> >>  return p ? max_count : p - string;
> > 
> > Richard's right, that's definitely a better implementation.
> 
> I was just wondering, what if we rewrote the code to use strlen()
> instead of strnlen(). Would that be an acceptable solution?

Only if you can do so safely - i.e. without accessing memory beyond
what we're supposed to.  I don't think you'll be able to do that
without effectively re-implementing strnlen(), there's a reason I used
it in the first place, after all.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[Peter Maydell]

On 21 October 2017 at 00:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 10/20/2017 10:55 AM, John Arbuckle wrote:
>> +static inline size_t strnlen(const char *string, size_t max_count)
>> +{
>> +    size_t count;
>> +    for (count = 0; count < max_count; count++) {
>> +        if (string[count] == '\0') {
>> +            break;
>> +        }
>> +    }
>> +    return count;
>
> Not to nitpick, but
>
>   const char *p = memchr(string, 0, max_count);
>   return p ? max_count : p - string;

Am I misreading that, or do you have the ?: arms the wrong way
around there?

thanks
-- PMM

Re: [Qemu-devel] [libfdt][PATCH v2] implement strnlen for systems that need it

[Programmingkid]

> On Oct 22, 2017, at 9:37 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> 
> On 21 October 2017 at 00:44, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>> On 10/20/2017 10:55 AM, John Arbuckle wrote:
>>> +static inline size_t strnlen(const char *string, size_t max_count)
>>> +{
>>> +    size_t count;
>>> +    for (count = 0; count < max_count; count++) {
>>> +        if (string[count] == '\0') {
>>> +            break;
>>> +        }
>>> +    }
>>> +    return count;
>> 
>> Not to nitpick, but
>> 
>>  const char *p = memchr(string, 0, max_count);
>>  return p ? max_count : p - string;
> 
> Am I misreading that, or do you have the ?: arms the wrong way
> around there?
> 
> thanks
> -- PMM

Yes. It should read:

return p ? p - string : max_count;