implement strnlen for systems that need it

PATCH Superseded John Arbuckle 3 participants 10 messages Oct 23, 2017 – Oct 25, 2017 7 messages not in the archive

Share & tools

John Arbuckle

author

2017-10-23 02:50 UTC #0 lore raw

PATCH v3 1/1 implement strnlen for systems that need it

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 <[email protected]>

---
v3 changes:
- Replaced loop with memchr()

v2 changes:
- Simplified the code to make it static inline'ed
- Changed the type of count to size_t

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

libfdt/libfdt_env.h +24 -0

index 952056c..d43b66b 100644
@@ -109,4 +109,28 @@ 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)
+{
+    const char *p = memchr(string, 0, max_count);
+    return p ? p - string : max_count;
+}
+
+#endif /* (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7) */
+
+#endif /* __APPLE__ */
+
 #endif /* _LIBFDT_ENV_H */

Signature · 1 line

2.10.2

Stefan Hajnoczi

2017-10-23 16:09 UTC #2 lore raw

In reply to a message not in the archive

Quoted 20 lines · 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 <[email protected]>
---
v3 changes:
- Replaced loop with memchr()
v2 changes:
- Simplified the code to make it static inline'ed
- Changed the type of count to size_t
 libfdt/libfdt_env.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
libfdt/libfdt_env.h +0 -0
index 952056c..d43b66b 100644

libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 115

Quoted 7 lines

libfdt/libfdt_env.h +4 -0

@@ -109,4 +109,28 @@ 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

Apple has already defined MAC_OS_X_VERSION_10_7 here:
https://opensource.apple.com/source/xnu/xnu-1699.24.8/EXTERNAL_HEADERS/AvailabilityMacros.h

To avoid a compiler warning, please use:

#ifndef MAC_OS_X_VERSION_10_7
#define MAC_OS_X_VERSION_10_7 1070
#endif

libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 118

Quoted 2 lines

libfdt/libfdt_env.h +2 -0

+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)

Does this cover the case where MAC_OS_X_VERSION_MAX_ALLOWED is set to <1070 on a 10.7+ build machine? It's possible that the <string.h> header would define strnlen() and your code redefines the function (compiler error).

It would be best to check how <string.h>, <Availability.h>, and <AvailabilityMacros.h> work to make sure that all cases are handled. I don't have access to a Mac right now, sorry.

Perhaps this approach works better:

# ifndef MAC_OS_X_VERSION_10_7

Peter Maydell

2017-10-23 16:27 UTC #4 lore raw

In reply to a message not in the archive

Quoted 7 lines · Stefan Hajnoczi

libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 118
Quoted 2 lines
libfdt/libfdt_env.h +2 -0
+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
Does this cover the case where MAC_OS_X_VERSION_MAX_ALLOWED is set to <1070 on a 10.7+ build machine? It's possible that the <string.h> header would define strnlen() and your code redefines the function (compiler error).

In that case you don't want to use the strnlen() declaration from the header, you want the inline somehow, because even if the declaration is present and using it doesn't fail compile the definition won't be around at runtime.

Quoted 3 lines

It would be best to check how <string.h>, <Availability.h>, and <AvailabilityMacros.h> work to make sure that all cases are handled. I don't have access to a Mac right now, sorry.

It uses the clang 'attribute availability' syntax:
https://clang.llvm.org/docs/AttributeReference.html#availability

thanks
-- PMM

Programmingkid

author

2017-10-24 03:13 UTC #12 lore raw

In reply to a message not in the archive

Quoted 35 lines · Stefan Hajnoczi

Quoted 20 lines · 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 <[email protected]>
---
v3 changes:
- Replaced loop with memchr()
v2 changes:
- Simplified the code to make it static inline'ed
- Changed the type of count to size_t
libfdt/libfdt_env.h | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
libfdt/libfdt_env.h +0 -0
index 952056c..d43b66b 100644
libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 115
Quoted 7 lines
libfdt/libfdt_env.h +4 -0
@@ -109,4 +109,28 @@ 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
Apple has already defined MAC_OS_X_VERSION_10_7 here:
https://opensource.apple.com/source/xnu/xnu-1699.24.8/EXTERNAL_HEADERS/AvailabilityMacros.h

Quoted 5 lines

To avoid a compiler warning, please use:
#ifndef MAC_OS_X_VERSION_10_7
#define MAC_OS_X_VERSION_10_7 1070
#endif

Sounds logical.

Quoted 8 lines

libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 118
Quoted 3 lines
libfdt/libfdt_env.h +2 -0
+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
Does this cover the case where MAC_OS_X_VERSION_MAX_ALLOWED is set to <1070 on a 10.7+ build machine? It's possible that the <string.h> header would define strnlen() and your code redefines the function (compiler error).

If MAC_OS_X_VERSION_MAX_ALLOWED is equal to 1070 then this code would not be executed. This branch would only be taken if MAC_OS_X_VERSION_MAX_ALLOWED is less than 1070. So there would not be a compiler error.

Quoted 8 lines

It would be best to check how <string.h>, <Availability.h>, and <AvailabilityMacros.h> work to make sure that all cases are handled. I don't have access to a Mac right now, sorry.
Perhaps this approach works better:
# ifndef MAC_OS_X_VERSION_10_7

I think you are saying I should remove the "#ifdef __APPLE__" code and just check to see if MAC_OS_X_VERSION_10_7 is defined. That might work on Mac OS 10.6 and 10.5, but it would probably cause problems with non-Mac-OS platforms.

Programmingkid

author

2017-10-24 03:45 UTC #6 lore raw

In reply to a message not in the archive

Quoted 12 lines · Peter Maydell

Quoted 7 lines · Stefan Hajnoczi
libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 118
Quoted 2 lines
libfdt/libfdt_env.h +2 -0
+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
Does this cover the case where MAC_OS_X_VERSION_MAX_ALLOWED is set to <1070 on a 10.7+ build machine? It's possible that the <string.h> header would define strnlen() and your code redefines the function (compiler error).

I was operating under the assumption that MAC_OS_X_VERSION_MAX_ALLOWED would equal the version of the host. After making this little test program:

#include <stdio.h>
#include <AvailabilityMacros.h>

int main(int argc, char *argv[])

{
    printf("value = %d\n", MAC_OS_X_VERSION_MAX_ALLOWED);
}

It reports:
"value = 101204" on Mac OS 10.12.6 (I'm not sure why there is a 04) and
"Value = 1068" on Mac OS 10.6.8

Is using MAC_OS_X_VERSION_MAX_ALLOWED not a reliable macro to use to test for the version of the Mac OS? The ui/cocoa.m file seems to use it and have no problems. I don't think we have to worry about MAC_OS_X_VERSION_MAX_ALLOWED being set to less than 1070 on Mac OS 10.7 and up.

Quoted 11 lines

In that case you don't want to use the strnlen() declaration from the header, you want the inline somehow, because even if the declaration is present and using it doesn't fail compile the definition won't be around at runtime.
Quoted 3 lines
It would be best to check how <string.h>, <Availability.h>, and <AvailabilityMacros.h> work to make sure that all cases are handled. I don't have access to a Mac right now, sorry.
It uses the clang 'attribute availability' syntax:
https://clang.llvm.org/docs/AttributeReference.html#availability

This feature appears to be a clang/gcc-only feature. Using it would mean making this code compiler locked. The Device Tree Compiler project (that this code belongs to) is made by IBM personnel. They might want to be able to use other compilers including their own IBM XL C compiler to compile this project. Even if that part of the code is only to run on Mac OS X I still would like to keep the code generic enough for any compiler to be able to build the Device Tree Compiler project.

Peter Maydell

2017-10-24 06:52 UTC #8 lore raw

In reply to a message not in the archive

Quoted 2 lines · Programmingkid

I was operating under the assumption that MAC_OS_X_VERSION_MAX_ALLOWED would equal the version of the host.

It indicates the highest version of OSX whose features the program being compiled is allowed to use. That isn't necessarily the same as the highest version of OSX the SDK we're compiling against supports -- you can be building against a 10.9-aware SDK and tell it "only use features up to and including those on 10.7."

Quoted 1 line

After making this little test program:

You don't need to test this kind of thing, it is documented, and in general reading the documentation is more reliable and more informative than testing.

Quoted 3 lines

Is using MAC_OS_X_VERSION_MAX_ALLOWED not a reliable macro to use to test for the version of the Mac OS? The ui/cocoa.m file seems to use it and have no problems.

QEMU's usage differs in two ways:
(1) we just use the macros to #ifdef out code that would be trying to use functions that don't exist in the older versions, which is exactly what it's intended for. We don't try to implement our own versions of those functions under the same names
(2) we control QEMU's build process and so we can guarantee that we aren't using the "build on a 10.9 SDK but make sure you don't use features that aren't in 10.7" functionality. libfdt is a library that can be included in many other programs, so we can't make that assumption.

Quoted 4 lines

Quoted 2 lines
It uses the clang 'attribute availability' syntax:
https://clang.llvm.org/docs/AttributeReference.html#availability
This feature appears to be a clang/gcc-only feature.

This was a reply to Stefan's question about how the OSX availability headers are implemented. (As it happens the OSX headers do have an "if not clang then just #define away the availability attributes" codepath.)

thanks
-- PMM

Stefan Hajnoczi

2017-10-24 12:09 UTC #14 lore raw

In reply to a message not in the archive

Quoted 59 lines · Programmingkid

Quoted 35 lines · Stefan Hajnoczi
Quoted 20 lines · 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 <[email protected]>
---
v3 changes:
- Replaced loop with memchr()
v2 changes:
- Simplified the code to make it static inline'ed
- Changed the type of count to size_t
libfdt/libfdt_env.h | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
libfdt/libfdt_env.h +0 -0
index 952056c..d43b66b 100644
libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 115
Quoted 7 lines
libfdt/libfdt_env.h +4 -0
@@ -109,4 +109,28 @@ 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
Apple has already defined MAC_OS_X_VERSION_10_7 here:
https://opensource.apple.com/source/xnu/xnu-1699.24.8/EXTERNAL_HEADERS/AvailabilityMacros.h
Quoted 5 lines
To avoid a compiler warning, please use:
#ifndef MAC_OS_X_VERSION_10_7
#define MAC_OS_X_VERSION_10_7 1070
#endif
Sounds logical.
Quoted 8 lines
libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 118
Quoted 3 lines
libfdt/libfdt_env.h +2 -0
+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
Does this cover the case where MAC_OS_X_VERSION_MAX_ALLOWED is set to <1070 on a 10.7+ build machine? It's possible that the <string.h> header would define strnlen() and your code redefines the function (compiler error).
If MAC_OS_X_VERSION_MAX_ALLOWED is equal to 1070 then this code would not be executed. This branch would only be taken if MAC_OS_X_VERSION_MAX_ALLOWED is less than 1070. So there would not be a compiler error.
Quoted 5 lines
It would be best to check how <string.h>, <Availability.h>, and <AvailabilityMacros.h> work to make sure that all cases are handled. I don't have access to a Mac right now, sorry.

Quoted 5 lines

Quoted 3 lines
Perhaps this approach works better:
# ifndef MAC_OS_X_VERSION_10_7
I think you are saying I should remove the "#ifdef __APPLE__" code and just check to see if MAC_OS_X_VERSION_10_7 is defined. That might work on Mac OS 10.6 and 10.5, but it would probably cause problems with non-Mac-OS platforms.

No, the ifndef is still "indented" and it would be like this:

#ifdef __APPLE_
...

# ifndef MAC_OS_X_VERSION_10_7

Stefan Hajnoczi

2017-10-24 12:18 UTC #9 lore raw

In reply to a message not in the archive

Quoted 9 lines · Peter Maydell

Quoted 7 lines · Stefan Hajnoczi
libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 118
Quoted 2 lines
libfdt/libfdt_env.h +2 -0
+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
Does this cover the case where MAC_OS_X_VERSION_MAX_ALLOWED is set to <1070 on a 10.7+ build machine? It's possible that the <string.h> header would define strnlen() and your code redefines the function (compiler error).

Quoted 4 lines

In that case you don't want to use the strnlen() declaration from the header, you want the inline somehow, because even if the declaration is present and using it doesn't fail compile the definition won't be around at runtime.

Perhaps one way around this case is to:

# if !defined(MAC_OS_X_VERSION_10_7) || (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
#  include <string.h> /* make sure it isn't included again */
#  define strnlen fdt_strnlen
static inline fdt_strnlen(...) { ... }

This way the symbol strnlen() isn't used and the system headers cannot cause problems.

Stefan

Programmingkid

author

2017-10-24 13:37 UTC #16 lore raw

In reply to a message not in the archive

Quoted 67 lines · Stefan Hajnoczi

Quoted 63 lines · Programmingkid
Quoted 35 lines · Stefan Hajnoczi
Quoted 20 lines · 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 <[email protected]>
---
v3 changes:
- Replaced loop with memchr()
v2 changes:
- Simplified the code to make it static inline'ed
- Changed the type of count to size_t
libfdt/libfdt_env.h | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
libfdt/libfdt_env.h +0 -0
index 952056c..d43b66b 100644
libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 115
Quoted 7 lines
libfdt/libfdt_env.h +4 -0
@@ -109,4 +109,28 @@ 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
Apple has already defined MAC_OS_X_VERSION_10_7 here:
https://opensource.apple.com/source/xnu/xnu-1699.24.8/EXTERNAL_HEADERS/AvailabilityMacros.h
Quoted 5 lines
To avoid a compiler warning, please use:
#ifndef MAC_OS_X_VERSION_10_7
#define MAC_OS_X_VERSION_10_7 1070
#endif
Sounds logical.
Quoted 8 lines
libfdt/libfdt_env.h static inline fdt64_t cpu_to_fdt64(uint64_t x) · line 117
Quoted 3 lines
libfdt/libfdt_env.h +2 -0
+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
Does this cover the case where MAC_OS_X_VERSION_MAX_ALLOWED is set to <1070 on a 10.7+ build machine? It's possible that the <string.h> header would define strnlen() and your code redefines the function (compiler error).
If MAC_OS_X_VERSION_MAX_ALLOWED is equal to 1070 then this code would not be executed. This branch would only be taken if MAC_OS_X_VERSION_MAX_ALLOWED is less than 1070. So there would not be a compiler error.
Quoted 8 lines
It would be best to check how <string.h>, <Availability.h>, and <AvailabilityMacros.h> work to make sure that all cases are handled. I don't have access to a Mac right now, sorry.
Perhaps this approach works better:
# ifndef MAC_OS_X_VERSION_10_7

Quoted 7 lines

Quoted 1 line
I think you are saying I should remove the "#ifdef __APPLE__" code and just check to see if MAC_OS_X_VERSION_10_7 is defined. That might work on Mac OS 10.6 and 10.5, but it would probably cause problems with non-Mac-OS platforms.
No, the ifndef is still "indented" and it would be like this:
#ifdef __APPLE_
...
# ifndef MAC_OS_X_VERSION_10_7

I see. This is a more compact way of doing things.

Programmingkid

author

2017-10-25 16:18 UTC #11 lore raw

In reply to a message not in the archive

Quoted 20 lines · Stefan Hajnoczi

Quoted 9 lines · Peter Maydell
Quoted 7 lines · Stefan Hajnoczi
Review
Quoted 2 lines
(quoted diff) +2 -0
+/* strnlen() is not available on Mac OS < 10.7 */
+# if (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
Does this cover the case where MAC_OS_X_VERSION_MAX_ALLOWED is set to <1070 on a 10.7+ build machine? It's possible that the <string.h> header would define strnlen() and your code redefines the function (compiler error).
Quoted 4 lines
In that case you don't want to use the strnlen() declaration from the header, you want the inline somehow, because even if the declaration is present and using it doesn't fail compile the definition won't be around at runtime.
Perhaps one way around this case is to:

Quoted 7 lines

# if !defined(MAC_OS_X_VERSION_10_7) || (MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_7)
#  include <string.h> /* make sure it isn't included again */
#  define strnlen fdt_strnlen
static inline fdt_strnlen(...) { ... }

This way the symbol strnlen() isn't used and the system headers cannot cause problems.

I really like this idea. I will implement it in my next patch. Thanks!