-
Notifications
You must be signed in to change notification settings - Fork 17
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
Ensure that BYTE_ORDER is set #538
Conversation
closes #534 |
@@ -35,6 +35,7 @@ set (LWIP_INCLUDE_DIRS | |||
include(${LWIP_DIR}/src/Filelists.cmake) | |||
|
|||
target_sources(lwipcore PRIVATE ${lwip_sys_srcs} lwip/lwiphooks_ip6.c lwip/lwiphooks_ip4.c lwip/lwip_cloned_fns.c) | |||
target_compile_definitions(lwipcore PUBLIC CMAKE_C_BYTE_ORDER=${CMAKE_C_BYTE_ORDER}) |
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.
I would not name it CMAKE_xxx
LWIP_BYTE_ORDER would be better
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.
why not just set BYTE_ORDER directly here?
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.
Mainly because the value of the cmake variable is either BIG_ENDIAN
or LITTLE_ENDIAN
, so we need to make sure those macros are defined as well but cmake doesn't provide them.
The
BYTE_ORDER
C preprocessor macro is needed by lwip, but the lwip includes (and -contrib packages) don't always include the correct system includes to pick up a definition ofBYTE_ORDER
. It turns out that there is no standard include file for gettingBYTE_ORDER
, so this PR relies on acmake
variable, but only ifBYTE_ORDER
is not already defined.