[rcl] Refactor error checking

[jacobperron]

• Remove redundant error checking
  • Checking for null pointers is delegated to the _is_valid() function (if such a function exists).
  • Removed null pointer check for "options" since it is not a pointer type.
• Change returned error codes to be more explicit
  • Replace RCL_RET_INVALID_ARGUMENT with RCL_RET_*_INVALID where possible.
  • Return RCL_RET_BAD_ALLOC where appropriate.

• Linux
• Linux-aarch64
• macOS
• Windows

[hidmic]

LGTM! Looks like I have some changes to apply to #319.

[hidmic]

@jacobperron wait sets still keep the fuzzy distinction between invalid argument and invalid entity, see here.

[wjwwood]

Is this ready for review?

[jacobperron]

Is this ready for review?

Sure :)

[wjwwood]

lgtm, other than one question

[wjwwood]

What's the rationale for removing the check of the options? Does it prevent this functions reuse somewhere else or something?

[jacobperron]

I thought it was redundant since options is not a pointer in the underlying impl struct, which is already checked in the line above.
The private function _client_get_options() returns the address, and the user-facing rcl_client_get_options() calls rcl_client_is_valid() anyways.

[wjwwood]

Makes sense, 👍

[jacobperron]

wait sets still keep the fuzzy distinction between invalid argument and invalid entity, see here.

@hidmic I was considering tackling wait sets, timers, and guard conditions later since they do not have the same usage pattern of _is_valid().