Add XXH_NAMESPACE, for namespace emulation in C #1065
Conversation
utilities/roslz4/CMakeLists.txt
Outdated
| PROPERTIES COMPILE_FLAGS "-Wno-sign-compare" COMPILE_DEFINITIONS "XXH_NAMESPACE=ROSLZ4_") | ||
| set_source_files_properties( | ||
| src/xxhash.c | ||
| PROPERTIES COMPILE_DEFINITIONS "XXH_NAMESPACE=ROSLZ4_") |
There was a problem hiding this comment.
Can you please set the compile definitions for both files in a single call:
set_source_files_properties(
src/lz4s.c src/xxhash.c
PROPERTIES COMPILE_DEFINITIONS "XXH_NAMESPACE=ROSLZ4_")
elemoine
force-pushed
the
xxh-namespace
branch
2 times, most recently
from
95426a1
to
8cf0966
Compare
utilities/roslz4/CMakeLists.txt
Outdated
| src/lz4s.c | ||
| PROPERTIES COMPILE_FLAGS "-Wno-sign-compare") | ||
| src/lz4s.c src/xxhash.c | ||
| PROPERTIES COMPILE_FLAGS "-Wno-sign-compare" COMPILE_DEFINITIONS "XXH_NAMESPACE=ROSLZ4_") |
There was a problem hiding this comment.
Please use a separate set_source_files_properties call since -Wno-sign-compare is not necessary for src/xxhash.c.
|
The patch looks good. Just waiting for CI... |
|
Thanks for reviewing. This issue has caused me major headhaches! |
|
@ros-pull-request-builder retest this please |
|
The build error doesn't seem to be related to my change. |
No worries, the failure is related to a different package. The problem has already been fixed but the Docker image is still using a previous version. It will start working in bit. |
|
@ros-pull-request-builder retest this please |
|
Thanks! |
ros/ros_comm#1065 was merged, so the master branch of ros_comm can now be used.
I've been trying to use
rosbagin a PostgreSQL extension based on Multicorn. But that doesn't work in PostgreSQL 9.6. I get SIGSEGV crashes. See https://github.com/elemoine/FdwRosTest for a complete test case with instructions on how to reproduce the bug.The issue is that the
postgresql9.6 binary uses liblz4, which, as theroslz4code, includes xxhash.h and xxhash.c. So when run in PostgreSQL the roslz4 code uses the XXH32_ symbols from liblz4 instead of its own symbols. This results in SIGSEGV crashes.This PR introduces the
XXH_NAMESPACEpre-processor variable for namespacing XXH32_ symbols. I did not invent anything – the latest xxHash code already uses that.Alternatively, the roslz4 code could be updated to include the latest xxHash code. But that would require changes to the lz4s.c code, as the xxHash API has changed.