From a8b880abadc3ceafdcc5a217e4327f4cd565b7d6 Mon Sep 17 00:00:00 2001 From: Andrei Kholodnyi Date: Fri, 15 Sep 2023 11:34:46 +0200 Subject: [PATCH 1/2] use strnlen_s instead of strnlen if available --- src/error_handling.c | 5 +++++ src/error_handling_helpers.h | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/error_handling.c b/src/error_handling.c index a0605373..ce99e66b 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -25,6 +25,7 @@ extern "C" #include #include #include +#define __STDC_WANT_LIB_EXT1__ 1 #include #include @@ -198,7 +199,11 @@ rcutils_set_error_state( error_state.line_number = line_number; #if RCUTILS_REPORT_ERROR_HANDLING_ERRORS // Only warn of overwritting if the new error is different from the old ones. +#ifdef __STDC_LIB_EXT1__ + size_t characters_to_compare = strnlen_s(error_string, RCUTILS_ERROR_MESSAGE_MAX_LENGTH); +#else size_t characters_to_compare = strnlen(error_string, RCUTILS_ERROR_MESSAGE_MAX_LENGTH); +#endif // assumption is that message length is <= max error string length static_assert( sizeof(gtls_rcutils_error_state.message) <= sizeof(gtls_rcutils_error_string.str), diff --git a/src/error_handling_helpers.h b/src/error_handling_helpers.h index 4914c4fe..07ef144a 100644 --- a/src/error_handling_helpers.h +++ b/src/error_handling_helpers.h @@ -36,6 +36,7 @@ #endif #include #include +#define __STDC_WANT_LIB_EXT1__ 1 #include #include @@ -130,7 +131,11 @@ __rcutils_convert_uint64_t_into_c_str(uint64_t number, char * buffer, size_t buf buffer[i] = '\0'; // reverse the string in place +#ifdef __STDC_LIB_EXT1__ + __rcutils_reverse_str(buffer, strnlen_s(buffer, 21)); +#else __rcutils_reverse_str(buffer, strnlen(buffer, 21)); +#endif } // do not use externally, internal function which is only to be used by error_handling.c From 6349f202a3c176fdc43bce3ac0c411c1626fffb9 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 7 Nov 2023 22:35:13 +0000 Subject: [PATCH 2/2] Implement rcutils_strnlen. This is a portable version that uses memchr as its underlying implementation. Signed-off-by: Chris Lalancette --- CMakeLists.txt | 3 +- include/rcutils/error_handling.h | 12 ++------ include/rcutils/strnlen.h | 44 ++++++++++++++++++++++++++++ src/error_handling.c | 8 ++--- src/error_handling_helpers.h | 8 ++--- src/strnlen.c | 24 +++++++++++++++ test/test_error_handling_helpers.cpp | 16 +++++----- 7 files changed, 85 insertions(+), 30 deletions(-) create mode 100644 include/rcutils/strnlen.h create mode 100644 src/strnlen.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 35d4df0c..7e004bbc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -72,6 +72,7 @@ set(rcutils_sources src/strerror.c src/string_array.c src/string_map.c + src/strnlen.c src/testing/fault_injection.c src/time.c ${time_impl_c} @@ -276,7 +277,7 @@ if(BUILD_TESTING) ) if(TARGET test_error_handling_helpers) target_include_directories(test_error_handling_helpers PUBLIC include) - target_link_libraries(test_error_handling_helpers osrf_testing_tools_cpp::memory_tools) + target_link_libraries(test_error_handling_helpers ${PROJECT_NAME} osrf_testing_tools_cpp::memory_tools) endif() ament_add_gtest(test_split diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index ee5667aa..bfc2e09d 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -22,9 +22,6 @@ extern "C" { #endif -#ifndef __STDC_WANT_LIB_EXT1__ -#define __STDC_WANT_LIB_EXT1__ 1 // indicate we would like strnlen_s if available -#endif #include #include #include @@ -36,23 +33,18 @@ extern "C" #include "rcutils/allocator.h" #include "rcutils/macros.h" #include "rcutils/snprintf.h" +#include "rcutils/strnlen.h" #include "rcutils/testing/fault_injection.h" #include "rcutils/types/rcutils_ret.h" #include "rcutils/visibility_control.h" -#ifdef __STDC_LIB_EXT1__ /// Write the given msg out to stderr, limiting the buffer size in the `fwrite`. /** * This ensures that there is an upper bound to a buffer overrun if `msg` is * non-null terminated. */ #define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) \ - do {fwrite(msg, sizeof(char), strnlen_s(msg, 4096), stderr);} while (0) -#else -/// Write the given msg out to stderr. -#define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) \ - do {fwrite(msg, sizeof(char), strlen(msg), stderr);} while (0) -#endif + do {fwrite(msg, sizeof(char), rcutils_strnlen(msg, 4096), stderr);} while (0) /// Set the error message to stderr using a format string and format arguments. /** diff --git a/include/rcutils/strnlen.h b/include/rcutils/strnlen.h new file mode 100644 index 00000000..82b6a5c8 --- /dev/null +++ b/include/rcutils/strnlen.h @@ -0,0 +1,44 @@ +// Copyright 2023 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/// \file + +#ifndef RCUTILS__STRNLEN_H_ +#define RCUTILS__STRNLEN_H_ + +#ifdef __cplusplus +extern "C" +{ +#endif + +#include "rcutils/macros.h" +#include "rcutils/visibility_control.h" + +/// Determine the length of a fixed-size string +/** + * \param[in] s Null terminated string to find the length of. + * \param[in] maxlen Maximum length to look to find the trailing \0. + * \return The length of the string if it is less than maxlen, or + * \return maxlen if there is no \0 among the first maxlen characters. + */ +RCUTILS_PUBLIC +RCUTILS_WARN_UNUSED +size_t +rcutils_strnlen(const char * s, size_t maxlen); + +#ifdef __cplusplus +} +#endif + +#endif // RCUTILS__STRNLEN_H_ diff --git a/src/error_handling.c b/src/error_handling.c index ce99e66b..7502499d 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -25,12 +25,12 @@ extern "C" #include #include #include -#define __STDC_WANT_LIB_EXT1__ 1 #include #include #include #include +#include // RCUTILS_REPORT_ERROR_HANDLING_ERRORS and RCUTILS_WARN_ON_TRUNCATION are set in the header below #include "./error_handling_helpers.h" @@ -199,11 +199,7 @@ rcutils_set_error_state( error_state.line_number = line_number; #if RCUTILS_REPORT_ERROR_HANDLING_ERRORS // Only warn of overwritting if the new error is different from the old ones. -#ifdef __STDC_LIB_EXT1__ - size_t characters_to_compare = strnlen_s(error_string, RCUTILS_ERROR_MESSAGE_MAX_LENGTH); -#else - size_t characters_to_compare = strnlen(error_string, RCUTILS_ERROR_MESSAGE_MAX_LENGTH); -#endif + size_t characters_to_compare = rcutils_strnlen(error_string, RCUTILS_ERROR_MESSAGE_MAX_LENGTH); // assumption is that message length is <= max error string length static_assert( sizeof(gtls_rcutils_error_state.message) <= sizeof(gtls_rcutils_error_string.str), diff --git a/src/error_handling_helpers.h b/src/error_handling_helpers.h index 07ef144a..70bdff24 100644 --- a/src/error_handling_helpers.h +++ b/src/error_handling_helpers.h @@ -36,10 +36,10 @@ #endif #include #include -#define __STDC_WANT_LIB_EXT1__ 1 #include #include +#include #ifdef __cplusplus extern "C" @@ -131,11 +131,7 @@ __rcutils_convert_uint64_t_into_c_str(uint64_t number, char * buffer, size_t buf buffer[i] = '\0'; // reverse the string in place -#ifdef __STDC_LIB_EXT1__ - __rcutils_reverse_str(buffer, strnlen_s(buffer, 21)); -#else - __rcutils_reverse_str(buffer, strnlen(buffer, 21)); -#endif + __rcutils_reverse_str(buffer, rcutils_strnlen(buffer, 21)); } // do not use externally, internal function which is only to be used by error_handling.c diff --git a/src/strnlen.c b/src/strnlen.c new file mode 100644 index 00000000..98721ea0 --- /dev/null +++ b/src/strnlen.c @@ -0,0 +1,24 @@ +// Copyright 2023 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "rcutils/strnlen.h" + +size_t +rcutils_strnlen(const char * s, size_t maxlen) +{ + const char * found = memchr(s, '\0', maxlen); + return found ? (size_t)(found - s) : maxlen; +} diff --git a/test/test_error_handling_helpers.cpp b/test/test_error_handling_helpers.cpp index 250c1b30..a304f4d3 100644 --- a/test/test_error_handling_helpers.cpp +++ b/test/test_error_handling_helpers.cpp @@ -18,6 +18,8 @@ #include "osrf_testing_tools_cpp/memory_tools/gtest_quickstart.hpp" +#include "rcutils/strnlen.h" + #include "../src/error_handling_helpers.h" TEST(test_error_handling, copy_string) { @@ -32,7 +34,7 @@ TEST(test_error_handling, copy_string) { written = __rcutils_copy_string(buffer, 3, "0123456789"); }); EXPECT_EQ(written, 2u); - EXPECT_EQ(strnlen(buffer, sizeof(buffer)), 2u); + EXPECT_EQ(rcutils_strnlen(buffer, sizeof(buffer)), 2u); EXPECT_STREQ(buffer, "01"); // normal truncation, 1 short of buffer length @@ -41,7 +43,7 @@ TEST(test_error_handling, copy_string) { written = __rcutils_copy_string(buffer, 9, "0123456789"); }); EXPECT_EQ(written, 8u); - EXPECT_EQ(strnlen(buffer, sizeof(buffer)), 8u); + EXPECT_EQ(rcutils_strnlen(buffer, sizeof(buffer)), 8u); EXPECT_STREQ(buffer, "01234567"); // input smaller than buffer, 1 short of buffer length @@ -50,7 +52,7 @@ TEST(test_error_handling, copy_string) { written = __rcutils_copy_string(buffer, 9, ""); }); EXPECT_EQ(written, 0u); - EXPECT_EQ(strnlen(buffer, sizeof(buffer)), 0u); + EXPECT_EQ(rcutils_strnlen(buffer, sizeof(buffer)), 0u); EXPECT_STREQ(buffer, ""); // copy where src and dst overlap (testing use of memmove vs memcpy) @@ -65,7 +67,7 @@ TEST(test_error_handling, copy_string) { written = __rcutils_copy_string(buffer, sizeof(buffer), buffer + 3); }); EXPECT_EQ(written, 6u); - EXPECT_EQ(strnlen(buffer, sizeof(buffer)), (sizeof(buffer) - 1) - 3); + EXPECT_EQ(rcutils_strnlen(buffer, sizeof(buffer)), (sizeof(buffer) - 1) - 3); EXPECT_STREQ(buffer, "456789"); } @@ -75,7 +77,7 @@ TEST(test_error_handling, reverse_str) { char buffer[] = "even"; EXPECT_NO_MEMORY_OPERATIONS( { - __rcutils_reverse_str(buffer, strnlen(buffer, sizeof(buffer))); + __rcutils_reverse_str(buffer, rcutils_strnlen(buffer, sizeof(buffer))); }); EXPECT_STREQ(buffer, "neve"); } @@ -83,7 +85,7 @@ TEST(test_error_handling, reverse_str) { char buffer[] = "reverseme"; EXPECT_NO_MEMORY_OPERATIONS( { - __rcutils_reverse_str(buffer, strnlen(buffer, sizeof(buffer))); + __rcutils_reverse_str(buffer, rcutils_strnlen(buffer, sizeof(buffer))); }); EXPECT_STREQ(buffer, "emesrever"); } @@ -91,7 +93,7 @@ TEST(test_error_handling, reverse_str) { char buffer[] = "a"; EXPECT_NO_MEMORY_OPERATIONS( { - __rcutils_reverse_str(buffer, strnlen(buffer, sizeof(buffer))); + __rcutils_reverse_str(buffer, rcutils_strnlen(buffer, sizeof(buffer))); }); EXPECT_STREQ(buffer, "a"); }