Skip to content

Commit

Permalink
use stack memory for rcutils_string_array_t
Browse files Browse the repository at this point in the history 
Signed-off-by: Chen Lihui <[email protected]>
  • Loading branch information
Chen Lihui committed Mar 18, 2022
1 parent da68d2b commit 8d7d286
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 50 deletions.
2 changes: 1 addition & 1 deletion rmw/include/rmw/subscription_content_filter_options.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ typedef struct RMW_PUBLIC_TYPE rmw_subscription_content_filter_options_s
* It can be NULL if there is no "%n" tokens placeholder in filter_expression.
* The maximum index number must be smaller than 100.
*/
rcutils_string_array_t * expression_parameters;
rcutils_string_array_t expression_parameters;
} rmw_subscription_content_filter_options_t;


Expand Down
52 changes: 15 additions & 37 deletions rmw/src/subscription_content_filter_options.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,10 @@
rmw_subscription_content_filter_options_t
rmw_get_zero_initialized_content_filter_options()
{
static rmw_subscription_content_filter_options_t zero_initialized_options = {
.filter_expression = NULL,
.expression_parameters = NULL,
};

return zero_initialized_options;
return (const rmw_subscription_content_filter_options_t) {
.filter_expression = NULL,
.expression_parameters = rcutils_get_zero_initialized_string_array()
}; // NOLINT(readability/braces): false positive
}

rmw_ret_t
Expand All @@ -48,7 +46,6 @@ rmw_subscription_content_filter_options_init(
rmw_ret_t ret = RMW_RET_OK;
rcutils_ret_t rcutils_ret;
char * new_filter_expression = NULL;
rcutils_string_array_t * new_expression_parameters = NULL;
size_t i;

new_filter_expression = rcutils_strdup(filter_expression, *allocator);
Expand All @@ -59,26 +56,18 @@ rmw_subscription_content_filter_options_init(
}

if (expression_parameters_argc > 0) {
new_expression_parameters =
allocator->allocate(sizeof(rcutils_string_array_t), allocator->state);
if (!new_expression_parameters) {
RMW_SET_ERROR_MSG("failed to allocate expression parameters");
ret = RMW_RET_BAD_ALLOC;
goto failed;
}
*new_expression_parameters = rcutils_get_zero_initialized_string_array();
rcutils_ret_t rcutils_ret = rcutils_string_array_init(
new_expression_parameters, expression_parameters_argc, allocator);
&options->expression_parameters, expression_parameters_argc, allocator);
if (RCUTILS_RET_OK != rcutils_ret) {
RMW_SET_ERROR_MSG("failed to init string array for expression parameters");
ret = RMW_RET_BAD_ALLOC;
goto failed;
}

for (i = 0; i < expression_parameters_argc; i++) {
new_expression_parameters->data[i] =
options->expression_parameters.data[i] =
rcutils_strdup(expression_parameter_argv[i], *allocator);
if (!new_expression_parameters->data[i]) {
if (!options->expression_parameters.data[i]) {
RMW_SET_ERROR_MSG("failed to copy expression parameter");
ret = RMW_RET_BAD_ALLOC;
goto clear_expression_parameters;
Expand All @@ -87,18 +76,16 @@ rmw_subscription_content_filter_options_init(
}

options->filter_expression = new_filter_expression;
options->expression_parameters = new_expression_parameters;

return RMW_RET_OK;

clear_expression_parameters:
rcutils_ret = rcutils_string_array_fini(new_expression_parameters);
rcutils_ret = rcutils_string_array_fini(&options->expression_parameters);
if (RCUTILS_RET_OK != rcutils_ret) {
RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to fini string array.\n");
}

failed:
allocator->deallocate(new_expression_parameters, allocator->state);
allocator->deallocate(new_filter_expression, allocator->state);

return ret;
Expand Down Expand Up @@ -135,21 +122,16 @@ rmw_subscription_content_filter_options_copy(
RMW_CHECK_ARGUMENT_FOR_NULL(src, RMW_RET_INVALID_ARGUMENT);
RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT);
RMW_CHECK_ARGUMENT_FOR_NULL(dst, RMW_RET_INVALID_ARGUMENT);
size_t expression_parameters_size = 0;
char ** expression_parameters_data = NULL;
if (src->expression_parameters) {
expression_parameters_size = src->expression_parameters->size;
expression_parameters_data = src->expression_parameters->data;
}

rmw_ret_t ret = rmw_subscription_content_filter_options_fini(dst, allocator);
if (ret != RMW_RET_OK) {
return ret;
}

return rmw_subscription_content_filter_options_init(
src->filter_expression,
expression_parameters_size,
(const char **)expression_parameters_data,
src->expression_parameters.size,
(const char **)src->expression_parameters.data,
allocator,
dst
);
Expand All @@ -168,14 +150,10 @@ rmw_subscription_content_filter_options_fini(
options->filter_expression = NULL;
}

if (options->expression_parameters) {
rcutils_ret_t ret = rcutils_string_array_fini(options->expression_parameters);
if (RCUTILS_RET_OK != ret) {
RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to fini string array.\n");
return RMW_RET_ERROR;
}
allocator->deallocate(options->expression_parameters, allocator->state);
options->expression_parameters = NULL;
rcutils_ret_t ret = rcutils_string_array_fini(&options->expression_parameters);
if (RCUTILS_RET_OK != ret) {
RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to fini string array.\n");
return RMW_RET_ERROR;
}

return RMW_RET_OK;
Expand Down
26 changes: 14 additions & 12 deletions rmw/test/test_subscription_content_filter_options.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,17 @@ TEST(rmw_subscription_content_filter_options, get_zero_init)
rmw_subscription_content_filter_options_t options =
rmw_get_zero_initialized_content_filter_options();
EXPECT_EQ(options.filter_expression, nullptr);
EXPECT_EQ(options.expression_parameters, nullptr);
EXPECT_EQ(options.expression_parameters.size, 0u);
EXPECT_EQ(options.expression_parameters.data, nullptr);
}

TEST(rmw_subscription_content_filter_options, options_init)
{
rmw_subscription_content_filter_options_t options =
rmw_get_zero_initialized_content_filter_options();
EXPECT_EQ(options.filter_expression, nullptr);
EXPECT_EQ(options.expression_parameters, nullptr);
EXPECT_EQ(options.expression_parameters.size, 0u);
EXPECT_EQ(options.expression_parameters.data, nullptr);

rcutils_allocator_t allocator = rcutils_get_default_allocator();
EXPECT_EQ(
Expand Down Expand Up @@ -77,10 +79,9 @@ TEST(rmw_subscription_content_filter_options, options_init)
&options));

EXPECT_STREQ(options.filter_expression, filter_expression);
ASSERT_NE(nullptr, options.expression_parameters);
ASSERT_EQ(expression_parameters_count, options.expression_parameters->size);
ASSERT_EQ(expression_parameters_count, options.expression_parameters.size);
for (size_t i = 0; i < expression_parameters_count; ++i) {
EXPECT_STREQ(options.expression_parameters->data[i], expression_parameters[i]);
EXPECT_STREQ(options.expression_parameters.data[i], expression_parameters[i]);
}

EXPECT_EQ(
Expand All @@ -93,7 +94,8 @@ TEST(rmw_subscription_content_filter_options, options_set)
rmw_subscription_content_filter_options_t options =
rmw_get_zero_initialized_content_filter_options();
EXPECT_EQ(options.filter_expression, nullptr);
EXPECT_EQ(options.expression_parameters, nullptr);
EXPECT_EQ(options.expression_parameters.size, 0u);
EXPECT_EQ(options.expression_parameters.data, nullptr);

rcutils_allocator_t allocator = rcutils_get_default_allocator();

Expand Down Expand Up @@ -139,10 +141,9 @@ TEST(rmw_subscription_content_filter_options, options_set)
&options));

EXPECT_STREQ(options.filter_expression, filter_expression);
ASSERT_NE(nullptr, options.expression_parameters);
ASSERT_EQ(expression_parameters_count, options.expression_parameters->size);
ASSERT_EQ(expression_parameters_count, options.expression_parameters.size);
for (size_t i = 0; i < expression_parameters_count; ++i) {
EXPECT_STREQ(options.expression_parameters->data[i], expression_parameters[i]);
EXPECT_STREQ(options.expression_parameters.data[i], expression_parameters[i]);
}

EXPECT_EQ(
Expand Down Expand Up @@ -185,7 +186,7 @@ TEST(rmw_subscription_content_filter_options, options_copy) {
rcutils_reset_error();

rcutils_allocator_t failing_allocator = get_time_bomb_allocator();
constexpr int expected_num_malloc_calls = 5;
constexpr int expected_num_malloc_calls = 4;
for (int i = 0; i < expected_num_malloc_calls; ++i) {
set_time_bomb_allocator_malloc_count(failing_allocator, i);
EXPECT_EQ(
Expand All @@ -204,7 +205,7 @@ TEST(rmw_subscription_content_filter_options, options_copy) {
EXPECT_EQ(
RCUTILS_RET_OK,
rcutils_string_array_cmp(
source.expression_parameters, destination.expression_parameters, &res));
&source.expression_parameters, &destination.expression_parameters, &res));
EXPECT_EQ(res, 0);

{
Expand All @@ -220,7 +221,8 @@ TEST(rmw_subscription_content_filter_options, options_copy) {
RMW_RET_OK,
rmw_subscription_content_filter_options_copy(&source2, &allocator, &destination));
EXPECT_STREQ(source2.filter_expression, destination.filter_expression);
EXPECT_EQ(nullptr, destination.expression_parameters);
EXPECT_EQ(source2.expression_parameters.size, 0u);
EXPECT_EQ(source2.expression_parameters.data, nullptr);
EXPECT_EQ(
RMW_RET_OK, rmw_subscription_content_filter_options_fini(&source2, &allocator));
}
Expand Down

0 comments on commit 8d7d286

Please sign in to comment.