Fix rcl_parse_yaml_file() error handling. #776
Conversation
It should not fini its output argument, silently invalidating the given pointer. Signed-off-by: Michel Hidalgo <[email protected]>
clalancette
left a comment
clalancette
left a comment
There was a problem hiding this comment.
Can you please clarify the contract of this method in the header file? It's not clear to me whether this is what is expected by downstream consumers.
Sure, we can add one. Considering that |
Signed-off-by: Michel Hidalgo <[email protected]>
|
@clalancette does 1ebb39a make sense? |
clalancette
left a comment
clalancette
left a comment
There was a problem hiding this comment.
Looks good, thanks for the additional clarification here.
| /// \brief Parse the YAML file, initialize and populate params_st | ||
| /// \brief Parse the YAML file and populate \p params_st | ||
| /// \pre Given \p params_st must be a valid parameter struct | ||
| /// as retured by `rcl_yaml_node_struct_init()` |
There was a problem hiding this comment.
| /// as retured by `rcl_yaml_node_struct_init()` | |
| /// as returned by `rcl_yaml_node_struct_init()` |
There was a problem hiding this comment.
Argh... 🤦 See be78d0d. Wonder why my spell checker didn't catch it.
Signed-off-by: Michel Hidalgo <[email protected]>
|
Alright, all green. Going in! |
|
Sorry I missed this, but this PR didn't adjust the unit tests to fix mem leaks. See #779 |
|
Will this need a backport? It changes API slightly. Backporting after I backport #754 will become much more difficult. However, not backporting this PR will cause a divergence that might introduce memory errors or leaks. |
It should not fini its output argument, silently invalidating the given pointer. Signed-off-by: Michel Hidalgo <[email protected]>
It should not fini its output argument, silently invalidating the given pointer. Signed-off-by: Michel Hidalgo <[email protected]>
It should not
finiits output argument, silently invalidating the given pointer.CI up to
rcl_yaml_param_parserandrcl: