-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OpenBSD: Use our own implementation of wcsnlen()if not available #11083
Conversation
I don't think this is the right approach, because this for-loop replaces the much more efficient SIMD implementations of these standard functions, which would be used otherwise. I think the real problem is, that |
This seems to be required to make our code compile with OpenBSD. See: https://mixxx.zulipchat.com/#narrow/stream/247620-development-help/topic/compiling.20mixxx.20on.20openbsd.20-current
I have no added a special case for OpenBSD only. Hopefully this solves the issue. |
I wonder if we've selected the C language version properly. Could you try set(CMAKE_C_STANDARD 11) or set(CMAKE_C_STANDARD 17) in CMakelists.txt |
src/util/string.h
Outdated
@@ -35,27 +35,40 @@ class StringCollator { | |||
/// A nullptr-safe variant of the corresponding standard C function. | |||
/// | |||
/// Treats nullptr like an empty string and returns 0. | |||
inline std::size_t strnlen( | |||
/// The c++11 strnlen_s() is not available on all targets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's C11, not C++11
This does not make any difference, because the C Standard is only set for c files, but we have only c++ files.
So that should be fine. I have no OpenBSD, so I cannot test. |
If we compile with |
These bound checking _s functions of the C11 standard seems to be rather controversial: https://stackoverflow.com/questions/50724726/why-didnt-gcc-or-glibc-implement-s-functions/50724865#50724865 They are official part of the standard (Annex K), but it's optional to implement them. The function wcsnlen (without _s) was never standardized. |
With this, I'm able to compile without getting an error on wcsnlen on OpenBSD. |
OK, Than this is ready for merge. @Swiftb0y can you have a final look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Feel free to squash or rebase or just merge yourself. |
This seems to be required to make our code compile with OpenBSD. See: https://mixxx.zulipchat.com/#narrow/stream/247620-development-help/topic/compiling.20mixxx.20on.20openbsd.20-current
It get also around the macro hell with MSVC/MingWG/wxWigets headers and avoids a redundant null check if actually the C++11 implementation is used.
Our implementation matches:
https://github.com/wxWidgets/wxWidgets/blob/fec8c06a53f5f2eb65e3328eadddd22334e7971f/include/wx/wxcrt.h#L183