diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d4a4736..d65c91a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,10 +1,6 @@ name: tests -on: - push: - branches: [master] - pull_request: - branches: [master] +on: [push] env: # Customize the CMake build type here (Release, Debug, RelWithDebInfo, etc.) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4bf94f8..2397e5b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,16 +5,31 @@ project(tyndall) set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) -set(C_CXX_EXTRA_FLAGS "-O2 -Wall -Wconversion -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -fconcepts-diagnostics-depth=4") +if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + set(C_CXX_EXTRA_FLAGS "-O2 -Wall -Wconversion -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -fconcepts-diagnostics-depth=4") +else() + set(C_CXX_EXTRA_FLAGS "-O2 -Wall -Wconversion -Wextra -Wno-unused-parameter -Wno-missing-field-initializers") +endif() + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${C_CXX_EXTRA_FLAGS}") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${C_CXX_EXTRA_FLAGS}") set(CMAKE_POSITION_INDEPENDENT_CODE ON) +if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") + add_compile_options(-Wno-gnu-string-literal-operator-template) +endif() + add_definitions(-DDEBUG) include_directories(${CMAKE_SOURCE_DIR}) -set(LD_FLAGS "-lpthread -lzmq -lprotobuf -lfmt -lrt -latomic") +set(LD_FLAGS "-lpthread") + +if(CMAKE_SYSTEM_NAME STREQUAL "Linux") + set(LD_FLAGS "${LD_FLAGS} -lrt -latomic") +endif() + +link_directories(${ZeroMQ_LIBRARY_DIRS}) find_package(ament_cmake) find_package(rclcpp) @@ -22,6 +37,11 @@ find_package(std_msgs) find_package(std_srvs) find_package(builtin_interfaces) find_package(Boost COMPONENTS program_options filesystem REQUIRED) +find_package(Protobuf REQUIRED) +find_package(fmt REQUIRED) + +find_package(PkgConfig REQUIRED) +pkg_check_modules(ZeroMQ REQUIRED libzmq) if("${rclcpp_FOUND}") message(STATUS "ROS2 found") @@ -33,7 +53,7 @@ endif() file(GLOB CFILES "tyndall/*.c*" "tyndall/*/*.c*") add_library(tyndall SHARED ${CFILES}) -target_link_libraries(tyndall ${LD_FLAGS}) +target_link_libraries(tyndall ${ZeroMQ_LIBRARIES} ${Protobuf_LIBRARIES} fmt::fmt ${LD_FLAGS}) install(TARGETS tyndall DESTINATION lib) if("${rclcpp_FOUND}") diff --git a/examples/meta.cpp b/examples/meta.cpp index 2a21884..c863219 100644 --- a/examples/meta.cpp +++ b/examples/meta.cpp @@ -19,7 +19,7 @@ int main() { printf("replace: %s\n", ("hei din sei"_strval).replace<'i', 'y'>().c_str()); printf("remove_leading: %s\n", ("///hei din sei"_strval).remove_leading<'/'>().c_str()); - printf("to_strval: %s\n", to_strval<42>::c_str()); + printf("to_strval: %s\n", to_strval<42>{}.c_str()); { printf("typevals:\n"); diff --git a/tests/ipc/ipc.cpp b/tests/ipc/ipc.cpp index a958201..0ad6983 100644 --- a/tests/ipc/ipc.cpp +++ b/tests/ipc/ipc.cpp @@ -42,6 +42,10 @@ int main() { { my_struct entry = {}; int rc = ipc_read(entry, "/test/standard"); + if (rc != 0) { + perror("ipc_read failed"); + std::cerr << "ipc_read returned error code: " << rc << std::endl; + } check(rc == 0); check(entry == ref); } diff --git a/tests/meta/strval.cpp b/tests/meta/strval.cpp index 02cd87b..71a4bde 100644 --- a/tests/meta/strval.cpp +++ b/tests/meta/strval.cpp @@ -4,20 +4,18 @@ #include #include + int main() { constexpr auto hei = "hei"_strval; - decltype(hei)::c_str(); - static_assert(hei.get<0>() == 'h'); - static_assert(hei.get<1>() == 'e'); - static_assert(hei.get<2>() == 'i'); + decltype(hei){}.c_str(); static_assert(hei.length() == 3); static_assert(hei + "du"_strval == "heidu"_strval); static_assert(("hei din sei"_strval).occurrences('i') == 3); static_assert(("a_b_c"_strval).replace<'_', '-'>() == "a-b-c"_strval); static_assert(("///hei"_strval).remove_leading<'/'>() == hei); static_assert(to_strval<42>{} == "42"_strval); - static_assert(sizeof(hei) == 3); - static_assert(sizeof(""_strval) == 0); + static_assert(sizeof(hei) == 4); + static_assert(sizeof(""_strval) == 1); { char buf[100]; @@ -27,4 +25,25 @@ int main() { *hei_p = {}; assert(strcmp(buf, hei.c_str()) == 0); } + + { + assert("abcdef"_strval.length() == 6); + assert("abcdef"_strval.occurrences('a') == 1); + assert("abcdef"_strval.occurrences('b') == 1); + assert("abcdef"_strval.occurrences('c') == 1); + assert("abcdef"_strval.occurrences('d') == 1); + assert("abcdef"_strval.occurrences('e') == 1); + assert("abcdef"_strval.occurrences('f') == 1); + assert("abcdef"_strval.occurrences('g') == 0); + } + + { + assert("/test/standard"_strval.length() == 14); + assert("/test/standard"_strval.occurrences('/') == 2); + assert("/test/standard"_strval.occurrences('t') == 3); + assert("/test/standard"_strval.occurrences('e') == 1); + assert("/test/standard"_strval.occurrences('s') == 2); + assert("/test/standard"_strval.occurrences('a') == 2); + + } } diff --git a/tests/reflect/reflect.cpp b/tests/reflect/reflect.cpp index 329ad0d..e9e83b1 100644 --- a/tests/reflect/reflect.cpp +++ b/tests/reflect/reflect.cpp @@ -78,7 +78,7 @@ int main() { static_assert(!std::is_aggregate_v && !std::is_scalar_v); static_assert(reflect().get_format() == ""_strval); - static_assert(sizeof(reflect().get_format()) == 0); + static_assert(sizeof(reflect().get_format()) == 1); static_assert(reflect().size() == 0); } } diff --git a/tyndall/ipc/id.h b/tyndall/ipc/id.h index e84e395..3ae81a0 100644 --- a/tyndall/ipc/id.h +++ b/tyndall/ipc/id.h @@ -4,20 +4,14 @@ #include #ifndef IPC_SHMEM_PREFIX -// echo ipc | sha1sum # more unique shared memory -// names reduce the chance of name collisions -#define IPC_SHMEM_PREFIX \ - "ipc" \ - "1ef42bc4e0bbfeb0ac34bc3642732768cf6f77b7" +#define IPC_SHMEM_PREFIX "tyn" #endif // Shared memory files all go in /dev/shm, so they need to be unique, and they // can't have folders. We take names that look like absolute paths, like // "/my-process/my_topic". We start with a prepend a constant unique prefix // (IPC_SHMEM_PREFIX), Then we add the id, without leading slashes, and with the -// remaining slashes replaced with underscores. And lastly, we append a hash of -// the the id without leading slashes, but with slashes. The last hash is to -// distinguish between eg. my/topic and my_topic. +// remaining slashes replaced with underscores. // Note that "my-topic" gets the same id as "/my-topic" and "//my-topic" etc. @@ -27,22 +21,15 @@ template using id_remove_leading_slashes = decltype(STRING::template remove_leading<'/'>()); -template -using id_hash = decltype(to_strval()>{}); - template using id_replace_slashes_with_underscores = - decltype(STRING::template replace<'/', '_'>()); + decltype(STRING::template replace<'/', '%'>()); template using id_prepare = decltype(create_strval(IPC_SHMEM_PREFIX) + "_"_strval + id_replace_slashes_with_underscores< id_remove_leading_slashes>{} - // switch slashes with underscores - + "_"_strval + id_hash>{} - // hash id to prevent name clash between f.ex. /my/topic and - // my_topic ); // runtime id generation @@ -52,13 +39,12 @@ static inline std::string id_rtid_prepare(const char *id) { while (*id == '/') ++id; - std::string prepared_id = IPC_SHMEM_PREFIX "_" + std::string{id} + "_" + - std::to_string(hash_fnv1a_32(id, strlen(id))); + std::string prepared_id = IPC_SHMEM_PREFIX "_" + std::string{id}; // replace slash with underscore for (char &c : prepared_id) if (c == '/') - c = '_'; + c = '%'; return prepared_id; } diff --git a/tyndall/ipc/shmem.h b/tyndall/ipc/shmem.h index e06789f..ba0aa3c 100644 --- a/tyndall/ipc/shmem.h +++ b/tyndall/ipc/shmem.h @@ -1,12 +1,19 @@ #pragma once -#include -#include + +#include +#include #include -#include -#include -#include +#include + +#include #include +#include #include +#include + +#ifdef __APPLE__ +#include "shmem_mac_ipc_names.h" +#endif enum shmem_error { SHMEM_SHM_FAILED = 1, @@ -14,20 +21,91 @@ enum shmem_error { SHMEM_MAP_FAILED, }; +#ifdef __APPLE__ +// macOS requires shm_open names to start with '/' +inline std::string fix_shm_name(const char *id) { + if (!id || id[0] == '\0') { + return {}; + } + std::string name(id); + if (name[0] != '/') { + name = "/" + name; + } + // macOS limit: name length <= 31 bytes + if (name.length() > 31) { + name = name.substr(0, 31); + } + return name; +} +#else +inline std::string fix_shm_name(const char *id) { + // Linux does not require starting slash, but having it is fine. + if (!id || id[0] == '\0') { + return {}; + } + return std::string(id); +} +#endif + static inline int shmem_create(void **addr, const char *id, size_t size) { - // memfd_create - int fd = shm_open(id, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR); - if (fd == -1) + if (!id || strlen(id) == 0) { + std::cerr << "shmem_create error: id is null or empty\n"; return SHMEM_SHM_FAILED; + } - int rc = ftruncate(fd, static_cast(size)); - if (rc != 0) - return SHMEM_TRUNCATE_FAILED; + std::string name = fix_shm_name(id); + if (name.empty()) { + std::cerr << "shmem_create error: fixed shm name is empty\n"; + return SHMEM_SHM_FAILED; + } - *addr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); - if (*addr == MAP_FAILED) + bool created = false; + int fd = shm_open(name.c_str(), O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR); + if (fd == -1 && errno == EEXIST) { + // Already exists, open without O_EXCL + fd = shm_open(name.c_str(), O_RDWR, S_IRUSR | S_IWUSR); + if (fd == -1) { + perror("shm_open"); + return SHMEM_SHM_FAILED; + } + } else if (fd != -1) { + created = true; + } else { + perror("shm_open"); + return SHMEM_SHM_FAILED; + } + + // Round size up to page size for mmap compatibility + size_t pagesize = static_cast(getpagesize()); + size_t rounded_size = ((size + pagesize - 1) / pagesize) * pagesize; + + if (created) { + #ifdef __APPLE__ + append_ipc_name(name); + #endif + + int rc = ftruncate(fd, static_cast(rounded_size)); + if (rc != 0) { + std::cerr << "shmem_create: ftruncate failed for fd "<< fd << " name " << name + << ", size = " << rounded_size << "\n"; + std::cerr << "ftruncate failed with errno = " << errno << ": " << strerror(errno) << "\n"; + + perror("ftruncate"); + close(fd); + return SHMEM_TRUNCATE_FAILED; + } + } + + void *map = mmap(NULL, rounded_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + if (map == MAP_FAILED) { + perror("mmap"); + close(fd); return SHMEM_MAP_FAILED; + } + + close(fd); + *addr = map; return 0; } @@ -36,32 +114,34 @@ static inline int shmem_unmap(void *addr, size_t size) { } static inline int shmem_unlink(const char *id) { - // shm_open cleanup - return shm_unlink(id); -} - -static inline int shmem_unlink_all(const char *prefix) { - DIR *dir; - struct dirent *ent; - dir = opendir("/dev/shm"); - if (dir == NULL) + if (!id || strlen(id) == 0) { return -1; - - while ((ent = readdir(dir)) != NULL) { - const char *dir_name = ent->d_name; - int match = 1; - - for (int i = 0; i < (int)strlen(prefix); ++i) - if (dir_name[i] != prefix[i]) - match = 0; - - if (match) - shmem_unlink(dir_name); } + std::string name = fix_shm_name(id); + if (name.empty()) { + return -1; + } + return shm_unlink(name.c_str()); +} - closedir(dir); - - return 0; +static inline int shmem_unlink_all(const char *prefix) { + #ifdef __APPLE__ + return unlink_all_ipc_names(); + #else + DIR *dir = opendir("/dev/shm"); + if (dir == nullptr) { + return -1; + } + struct dirent *ent; + while ((ent = readdir(dir)) != nullptr) { + const char *dir_name = ent->d_name; + if (strncmp(dir_name, prefix, strlen(prefix)) == 0) { + shmem_unlink(dir_name); + } + } + closedir(dir); + return 0; + #endif } #ifdef __cplusplus @@ -133,9 +213,10 @@ class shmem_buf { public: shmem_buf() noexcept { static_assert(ID::occurrences('/') == 0, "Id can't have slashes"); - if constexpr (ID{} != ""_strval) - init(ID::c_str()); + { + init(ID{}.c_str()); + } } shmem_buf(const char *id) noexcept { @@ -271,4 +352,4 @@ class shmem_buf { } }; -#endif +#endif // __cplusplus diff --git a/tyndall/ipc/shmem_mac_ipc_names.h b/tyndall/ipc/shmem_mac_ipc_names.h new file mode 100644 index 0000000..4edb4ad --- /dev/null +++ b/tyndall/ipc/shmem_mac_ipc_names.h @@ -0,0 +1,89 @@ +#pragma once + +#ifdef __APPLE__ + +#include +#include +#include +#include +#include +#include +#include + +static const char *ipc_names_file = "/tmp/tyndall_ipc_names"; + +inline void append_ipc_name(const std::string &name) { + int fd = open(ipc_names_file, O_RDWR | O_CREAT, 0666); + if (fd == -1) { + perror("open ipc_names_file"); + return; + } + if (flock(fd, LOCK_EX) == -1) { + perror("flock"); + close(fd); + return; + } + + std::set names; + { + std::ifstream infile(ipc_names_file); + std::string line; + while (std::getline(infile, line)) { + names.insert(line); + } + } + + if (names.find(name) == names.end()) { + names.insert(name); + std::ofstream outfile(ipc_names_file, std::ios::trunc); + for (const auto &n : names) { + outfile << n << "\n"; + } + } + + flock(fd, LOCK_UN); + close(fd); +} + +inline int unlink_all_ipc_names() { + int fd = open(ipc_names_file, O_RDWR); + if (fd == -1) { + return 0; // no file, nothing to unlink + } + + if (flock(fd, LOCK_EX) == -1) { + perror("flock"); + close(fd); + return -1; + } + + std::set names_to_unlink; + std::set names_to_keep; + + { + std::ifstream infile(ipc_names_file); + std::string line; + while (std::getline(infile, line)) { + names_to_unlink.insert(line); + } + } + + for (const auto &name : names_to_unlink) { + if (shm_unlink(name.c_str()) != 0) { + perror(("shm_unlink " + name).c_str()); + } + } + + { + std::ofstream outfile(ipc_names_file, std::ios::trunc); + for (const auto &n : names_to_keep) { + outfile << n << "\n"; + } + } + + flock(fd, LOCK_UN); + close(fd); + return 0; +} + +#endif // __APPLE__ diff --git a/tyndall/meta/strval.h b/tyndall/meta/strval.h index f128d8a..85bcf40 100644 --- a/tyndall/meta/strval.h +++ b/tyndall/meta/strval.h @@ -9,13 +9,15 @@ template struct strval {}; template struct strval { + char data[1 + sizeof...(Rhs) + 1] = {Lhs, Rhs..., '\0'}; // storage + null terminator + template constexpr auto operator+(strval rhs) const noexcept { return strval(); } - static constexpr const char *c_str() noexcept { - return (const char[]){Lhs, Rhs..., '\0'}; + constexpr const char* c_str() const noexcept { + return data; } static constexpr int length() noexcept { return 1 + sizeof...(Rhs); } @@ -56,20 +58,20 @@ template struct strval { data[i] = tmp[i]; } - char data[length()] = {Lhs, Rhs...}; // actual storage - constexpr static inline bool is_strval() { return true; } }; template <> struct strval<> { - char data[0]; // enforce zero size + char data[1] = {'\0'}; // empty string with null terminator template constexpr auto operator+(strval args) const noexcept { return strval(); } - static constexpr const char *c_str() noexcept { return (const char[]){'\0'}; } + constexpr const char* c_str() const noexcept { + return data; + } static constexpr int length() noexcept { return 0; } diff --git a/tyndall/meta/typevals.h b/tyndall/meta/typevals.h index 585c036..ed82491 100644 --- a/tyndall/meta/typevals.h +++ b/tyndall/meta/typevals.h @@ -5,7 +5,23 @@ typevals is a container with entries that have different types */ -template class typevals {}; +template +class typevals {}; + +// Helper: get type at index in Types pack +template +struct type_at; + +template +struct type_at<0, Head, Tail...> { + using type = Head; +}; + +template +struct type_at { + static_assert(Index < sizeof...(Tail) + 1, "Index out of bounds"); + using type = typename type_at::type; +}; template struct typevals : public typevals { @@ -21,43 +37,34 @@ struct typevals : public typevals { } template - requires(index == sizeof...(Tail)) - constexpr const Type &get() const noexcept { - return val; - } - - template - requires(index < sizeof...(Tail)) - constexpr decltype(typevals::template get_type()) - get() const noexcept { - return typevals::template get(); + constexpr decltype(auto) get() const noexcept { + if constexpr (index == sizeof...(Tail)) { + return val; + } else { + return typevals::template get(); + } } template - constexpr decltype(typevals::template get_type()) - operator[](std::integral_constant) const noexcept { + constexpr decltype(auto) operator[](std::integral_constant) const noexcept { return get(); } static constexpr int size() noexcept { return 1 + sizeof...(Tail); } protected: - // get_type is a static helper for determining return type of get template - requires(index == sizeof...(Tail)) - static constexpr const Type &get_type() noexcept { - return Type(); - } - - template - requires(index < sizeof...(Tail)) - static constexpr decltype(typevals::template get_type()) - get_type() noexcept { - return typevals::template get_type(); + static constexpr typename type_at::type get_type() noexcept { + if constexpr (index == sizeof...(Tail)) { + return Type(); + } else { + return typevals::template get_type(); + } } }; -template <> struct typevals<> { +template <> +struct typevals<> { explicit constexpr typevals() noexcept {} template @@ -67,6 +74,16 @@ template <> struct typevals<> { static constexpr int size() noexcept { return 0; } + template + constexpr decltype(auto) get() const noexcept { + static_assert(index < 0, "Index out of bounds in typevals"); + // This will always trigger a compile error if called + } + protected: - template static constexpr int get_type() noexcept { return 0; } + template + static constexpr int get_type() noexcept { + static_assert(index < 0, "Index out of bounds in typevals"); + return 0; + } };