Skip to content

Commit 6b5932c

Browse files
authored
Cleanup error handling in rcutils. (ros2#485)
This gets rid of ugly warnings about overwritten errors, both in the tests and in downstream code. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
1 parent f662cca commit 6b5932c

6 files changed

Lines changed: 16 additions & 6 deletions

File tree

src/char_array.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ rcutils_char_array_vsprintf(rcutils_char_array_t * char_array, const char * form
172172
if (new_size > char_array->buffer_capacity) {
173173
rcutils_ret_t ret = rcutils_char_array_expand_as_needed(char_array, new_size);
174174
if (ret != RCUTILS_RET_OK) {
175-
RCUTILS_SET_ERROR_MSG("char array failed to expand");
175+
// rcutils_char_array_expand_as_needed already set the error
176176
return ret;
177177
}
178178

@@ -196,7 +196,7 @@ rcutils_char_array_memcpy(rcutils_char_array_t * char_array, const char * src, s
196196
{
197197
rcutils_ret_t ret = rcutils_char_array_expand_as_needed(char_array, n);
198198
if (ret != RCUTILS_RET_OK) {
199-
RCUTILS_SET_ERROR_MSG("char array failed to expand");
199+
// rcutils_char_array_expand_as needed already set the error
200200
return ret;
201201
}
202202
memcpy(char_array->buffer, src, n);
@@ -223,7 +223,7 @@ rcutils_char_array_strncat(rcutils_char_array_t * char_array, const char * src,
223223
size_t new_length = current_strlen + n + 1;
224224
rcutils_ret_t ret = rcutils_char_array_expand_as_needed(char_array, new_length);
225225
if (ret != RCUTILS_RET_OK) {
226-
RCUTILS_SET_ERROR_MSG("char array failed to expand");
226+
// rcutils_char_array_expand_as_needed already set the error
227227
return ret;
228228
}
229229

src/logging.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,7 @@ rcutils_ret_t rcutils_logging_initialize_with_allocator(rcutils_allocator_t allo
748748
&g_rcutils_logging_severities_map, 2, sizeof(const char *), sizeof(int),
749749
rcutils_hash_map_string_hash_func, rcutils_hash_map_string_cmp_func, &allocator);
750750
if (hash_map_ret != RCUTILS_RET_OK) {
751-
// If an error message was set it will have been overwritten by rcutils_hash_map_init.
751+
rcutils_reset_error();
752752
RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(
753753
"Failed to initialize map for logger severities [%s]. Severities will not be configurable.",
754754
rcutils_get_error_string().str);

src/split.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,10 @@ rcutils_split(
6363
++array_size;
6464
}
6565
}
66-
if (rcutils_string_array_init(string_array, array_size, &allocator) != RCUTILS_RET_OK) {
67-
goto fail;
66+
rcutils_ret_t ret = rcutils_string_array_init(string_array, array_size, &allocator);
67+
if (ret != RCUTILS_RET_OK) {
68+
// rcutils_string_array_init already set the error
69+
return ret;
6870
}
6971

7072
size_t token_counter = 0;

test/test_hash_map.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,15 @@ TEST_F(HashMapBaseTest, init_map_failing_allocator) {
150150
&map, 2, sizeof(uint32_t), sizeof(uint32_t),
151151
test_hash_map_uint32_hash_func, test_uint32_cmp, &failing_allocator);
152152
EXPECT_EQ(RCUTILS_RET_BAD_ALLOC, ret) << rcutils_get_error_string().str;
153+
rcutils_reset_error();
153154

154155
// Check allocate hash_map->impl->map fails
155156
set_time_bomb_allocator_malloc_count(failing_allocator, 1);
156157
ret = rcutils_hash_map_init(
157158
&map, 2, sizeof(uint32_t), sizeof(uint32_t),
158159
test_hash_map_uint32_hash_func, test_uint32_cmp, &failing_allocator);
159160
EXPECT_EQ(RCUTILS_RET_BAD_ALLOC, ret) << rcutils_get_error_string().str;
161+
rcutils_reset_error();
160162
}
161163

162164
TEST_F(HashMapBaseTest, init_map_success) {

test/test_logging.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,14 @@ TEST(TestLogging, test_logging_initialization) {
4242
rcutils_allocator_t empty_allocator = rcutils_get_zero_initialized_allocator();
4343
EXPECT_EQ(
4444
RCUTILS_RET_INVALID_ARGUMENT, rcutils_logging_initialize_with_allocator(empty_allocator));
45+
rcutils_reset_error();
4546

4647
// Testing with a bad allocator fails when allocating internal memory
4748
// for the string map relating severity level values to string
4849
rcutils_allocator_t failing_allocator = get_failing_allocator();
4950
EXPECT_EQ(
5051
RCUTILS_RET_ERROR, rcutils_logging_initialize_with_allocator(failing_allocator));
52+
rcutils_reset_error();
5153
}
5254

5355
size_t g_log_calls = 0;

test/test_split.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,27 +56,31 @@ TEST(test_split, split) {
5656
EXPECT_EQ(
5757
RCUTILS_RET_INVALID_ARGUMENT,
5858
rcutils_split("Test", '/', rcutils_get_default_allocator(), NULL));
59+
rcutils_reset_error();
5960

6061
// Allocating initial string_array fails
6162
rcutils_allocator_t time_bomb_allocator = get_time_bomb_allocator();
6263
set_time_bomb_allocator_calloc_count(time_bomb_allocator, 0);
6364
EXPECT_EQ(
6465
RCUTILS_RET_BAD_ALLOC,
6566
rcutils_split("Test", '/', time_bomb_allocator, &tokens_fail));
67+
rcutils_reset_error();
6668

6769
// Allocating string_array->data fails
6870
set_time_bomb_allocator_calloc_count(time_bomb_allocator, 1);
6971
set_time_bomb_allocator_malloc_count(time_bomb_allocator, 0);
7072
EXPECT_EQ(
7173
RCUTILS_RET_BAD_ALLOC,
7274
rcutils_split("Test", '/', time_bomb_allocator, &tokens_fail));
75+
rcutils_reset_error();
7376

7477
// Allocating string_array->data[0] fails
7578
set_time_bomb_allocator_calloc_count(time_bomb_allocator, 1);
7679
set_time_bomb_allocator_malloc_count(time_bomb_allocator, 1);
7780
EXPECT_EQ(
7881
RCUTILS_RET_BAD_ALLOC,
7982
rcutils_split("hello/world", '/', time_bomb_allocator, &tokens_fail));
83+
rcutils_reset_error();
8084

8185
rcutils_string_array_t tokens0 = test_split("", '/', 0);
8286
ret = rcutils_string_array_fini(&tokens0);

0 commit comments

Comments
 (0)