-
Notifications
You must be signed in to change notification settings - Fork 181
Refactor parser.c for better testability #754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
a3718fa
Refactor rcl_yaml_param_parser for better testability
brawner f22c397
Reorder parser.c to match parser.h
brawner 33bcec2
Refactor yaml_variant.c for deduplication
brawner 146c467
ADD_VALUE_TO_SIMPLE_ARRAY for deduplication
brawner 7098edd
Remove fprintf
brawner 643beda
PR Fixup
brawner 705d230
Move headers to src directory
brawner 2abe117
PR Fixup
brawner 8d72dc4
Rebase #780
brawner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| // Copyright 2018 Apex.AI, 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 "./impl/add_to_arrays.h" | ||
|
|
||
| #define ADD_VALUE_TO_SIMPLE_ARRAY(val_array, value, value_type, allocator) \ | ||
| do { \ | ||
| if (NULL == val_array->values) { \ | ||
| val_array->values = value; \ | ||
| val_array->size = 1; \ | ||
| } else { \ | ||
| /* Increase the array size by one and add the new value */ \ | ||
| value_type * tmp_arr = val_array->values; \ | ||
| val_array->values = allocator.zero_allocate( \ | ||
| val_array->size + 1U, sizeof(value_type), allocator.state); \ | ||
| if (NULL == val_array->values) { \ | ||
| val_array->values = tmp_arr; \ | ||
| RCUTILS_SAFE_FWRITE_TO_STDERR("Error allocating mem\n"); \ | ||
| return RCUTILS_RET_BAD_ALLOC; \ | ||
| } \ | ||
| memcpy(val_array->values, tmp_arr, (val_array->size * sizeof(value_type))); \ | ||
| val_array->values[val_array->size] = *value; \ | ||
| val_array->size++; \ | ||
| allocator.deallocate(value, allocator.state); \ | ||
| allocator.deallocate(tmp_arr, allocator.state); \ | ||
| } \ | ||
| return RCUTILS_RET_OK; \ | ||
| } while (0) | ||
|
|
||
| rcutils_ret_t add_val_to_bool_arr( | ||
| rcl_bool_array_t * const val_array, | ||
| bool * value, | ||
| const rcutils_allocator_t allocator) | ||
| { | ||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(val_array, RCUTILS_RET_INVALID_ARGUMENT); | ||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT); | ||
| RCUTILS_CHECK_ALLOCATOR_WITH_MSG( | ||
| &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); | ||
|
|
||
| ADD_VALUE_TO_SIMPLE_ARRAY(val_array, value, bool, allocator); | ||
| } | ||
|
|
||
| /// | ||
| /// Add a value to an integer array. Create the array if it does not exist | ||
| /// | ||
| rcutils_ret_t add_val_to_int_arr( | ||
| rcl_int64_array_t * const val_array, | ||
| int64_t * value, | ||
| const rcutils_allocator_t allocator) | ||
| { | ||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(val_array, RCUTILS_RET_INVALID_ARGUMENT); | ||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT); | ||
| RCUTILS_CHECK_ALLOCATOR_WITH_MSG( | ||
| &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); | ||
|
|
||
| ADD_VALUE_TO_SIMPLE_ARRAY(val_array, value, int64_t, allocator); | ||
| } | ||
|
|
||
| /// | ||
| /// Add a value to a double array. Create the array if it does not exist | ||
| /// | ||
| rcutils_ret_t add_val_to_double_arr( | ||
| rcl_double_array_t * const val_array, | ||
| double * value, | ||
| const rcutils_allocator_t allocator) | ||
| { | ||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(val_array, RCUTILS_RET_INVALID_ARGUMENT); | ||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT); | ||
| RCUTILS_CHECK_ALLOCATOR_WITH_MSG( | ||
| &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); | ||
|
|
||
| ADD_VALUE_TO_SIMPLE_ARRAY(val_array, value, double, allocator); | ||
| } | ||
|
|
||
| /// | ||
| /// Add a value to a string array. Create the array if it does not exist | ||
| /// | ||
| rcutils_ret_t add_val_to_string_arr( | ||
| rcutils_string_array_t * const val_array, | ||
| char * value, | ||
| const rcutils_allocator_t allocator) | ||
| { | ||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(val_array, RCUTILS_RET_INVALID_ARGUMENT); | ||
| RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT); | ||
| RCUTILS_CHECK_ALLOCATOR_WITH_MSG( | ||
| &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); | ||
|
|
||
| if (NULL == val_array->data) { | ||
| rcutils_ret_t ret = rcutils_string_array_init(val_array, 1, &allocator); | ||
| if (RCUTILS_RET_OK != ret) { | ||
| return ret; | ||
| } | ||
| val_array->data[0U] = value; | ||
| } else { | ||
| /// Increase the array size by one and add the new value | ||
| char ** new_string_arr_ptr = allocator.reallocate( | ||
| val_array->data, | ||
| ((val_array->size + 1U) * sizeof(char *)), allocator.state); | ||
| if (NULL == new_string_arr_ptr) { | ||
| RCUTILS_SAFE_FWRITE_TO_STDERR("Error allocating mem\n"); | ||
| return RCUTILS_RET_BAD_ALLOC; | ||
brawner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| val_array->data = new_string_arr_ptr; | ||
| val_array->data[val_array->size] = value; | ||
| val_array->size++; | ||
| } | ||
| return RCUTILS_RET_OK; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| // Copyright 2018 Apex.AI, 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. | ||
|
|
||
| #ifndef IMPL__ADD_TO_ARRAYS_H_ | ||
| #define IMPL__ADD_TO_ARRAYS_H_ | ||
|
|
||
| #include <yaml.h> | ||
|
|
||
| #include "rcutils/types.h" | ||
|
|
||
| #include "./types.h" | ||
| #include "rcl_yaml_param_parser/types.h" | ||
| #include "rcl_yaml_param_parser/visibility_control.h" | ||
|
|
||
| #ifdef __cplusplus | ||
| extern "C" | ||
| { | ||
| #endif | ||
|
|
||
| /// | ||
| /// Add a value to a bool array. Create the array if it does not exist | ||
| /// | ||
| RCL_YAML_PARAM_PARSER_PUBLIC | ||
| RCUTILS_WARN_UNUSED | ||
| rcutils_ret_t add_val_to_bool_arr( | ||
| rcl_bool_array_t * const val_array, | ||
| bool * value, | ||
| const rcutils_allocator_t allocator); | ||
|
|
||
| /// | ||
| /// Add a value to an integer array. Create the array if it does not exist | ||
| /// | ||
| RCL_YAML_PARAM_PARSER_PUBLIC | ||
| RCUTILS_WARN_UNUSED | ||
| rcutils_ret_t add_val_to_int_arr( | ||
| rcl_int64_array_t * const val_array, | ||
| int64_t * value, | ||
| const rcutils_allocator_t allocator); | ||
|
|
||
| /// | ||
| /// Add a value to a double array. Create the array if it does not exist | ||
| /// | ||
| RCL_YAML_PARAM_PARSER_PUBLIC | ||
| RCUTILS_WARN_UNUSED | ||
| rcutils_ret_t add_val_to_double_arr( | ||
| rcl_double_array_t * const val_array, | ||
| double * value, | ||
| const rcutils_allocator_t allocator); | ||
|
|
||
| /// | ||
| /// Add a value to a string array. Create the array if it does not exist | ||
| /// | ||
| RCL_YAML_PARAM_PARSER_PUBLIC | ||
| RCUTILS_WARN_UNUSED | ||
| rcutils_ret_t add_val_to_string_arr( | ||
| rcutils_string_array_t * const val_array, | ||
| char * value, | ||
| const rcutils_allocator_t allocator); | ||
|
|
||
| /// | ||
| /// TODO (anup.pemmaiah): Support byte array | ||
| /// | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| #endif // IMPL__ADD_TO_ARRAYS_H_ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| // Copyright 2018 Apex.AI, 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. | ||
|
|
||
| #ifndef IMPL__NAMESPACE_H_ | ||
| #define IMPL__NAMESPACE_H_ | ||
|
|
||
| #include <yaml.h> | ||
|
|
||
| #include "rcutils/types.h" | ||
|
|
||
| #include "./types.h" | ||
| #include "rcl_yaml_param_parser/types.h" | ||
| #include "rcl_yaml_param_parser/visibility_control.h" | ||
|
|
||
| #ifdef __cplusplus | ||
| extern "C" | ||
| { | ||
| #endif | ||
|
|
||
| /// | ||
| /// Add name to namespace tracker | ||
| /// | ||
| RCL_YAML_PARAM_PARSER_PUBLIC | ||
| RCUTILS_WARN_UNUSED | ||
| rcutils_ret_t add_name_to_ns( | ||
| namespace_tracker_t * ns_tracker, | ||
| const char * name, | ||
| const namespace_type_t namespace_type, | ||
| rcutils_allocator_t allocator); | ||
|
|
||
| /// | ||
| /// Remove name from namespace tracker | ||
| /// | ||
| RCL_YAML_PARAM_PARSER_PUBLIC | ||
| RCUTILS_WARN_UNUSED | ||
| rcutils_ret_t rem_name_from_ns( | ||
| namespace_tracker_t * ns_tracker, | ||
| const namespace_type_t namespace_type, | ||
| rcutils_allocator_t allocator); | ||
|
|
||
| /// | ||
| /// Replace namespace in namespace tracker | ||
| /// | ||
| RCL_YAML_PARAM_PARSER_PUBLIC | ||
| RCUTILS_WARN_UNUSED | ||
| rcutils_ret_t replace_ns( | ||
| namespace_tracker_t * ns_tracker, | ||
| char * const new_ns, | ||
| const uint32_t new_ns_count, | ||
| const namespace_type_t namespace_type, | ||
| rcutils_allocator_t allocator); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| #endif // IMPL__NAMESPACE_H_ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| // Copyright 2018 Apex.AI, 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. | ||
|
|
||
| #ifndef IMPL__NODE_PARAMS_H_ | ||
| #define IMPL__NODE_PARAMS_H_ | ||
|
|
||
| #include "rcl_yaml_param_parser/types.h" | ||
| #include "rcl_yaml_param_parser/visibility_control.h" | ||
|
|
||
| #ifdef __cplusplus | ||
| extern "C" | ||
| { | ||
| #endif | ||
|
|
||
| /// | ||
| /// Create rcl_node_params_t structure | ||
| /// | ||
| RCL_YAML_PARAM_PARSER_PUBLIC | ||
| RCUTILS_WARN_UNUSED | ||
| rcutils_ret_t node_params_init( | ||
| rcl_node_params_t * node_params, | ||
| const rcutils_allocator_t allocator); | ||
|
|
||
| /// | ||
| /// Finalize rcl_node_params_t structure | ||
| /// | ||
| RCL_YAML_PARAM_PARSER_PUBLIC | ||
| void rcl_yaml_node_params_fini( | ||
| rcl_node_params_t * node_params, | ||
| const rcutils_allocator_t allocator); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| #endif // IMPL__NODE_PARAMS_H_ |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brawner nit: if
ADD_VALUE_TO_SIMPLE_ARRAYtook ownership ofvalueand deallocated it in failure cases,add_val_to_*API callers wouldn't have to add a trailing nullity check and deallocation every time one of those functions fails.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The callers already have to handle deallocation of value in several instances. The logic is complicated and this change will lead to larger changes, which I think are out of scope for just adding coverage to this package.