Skip to content
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

Add fault injection macros for use in other packages #254

Merged
merged 4 commits into from
Aug 31, 2020

Conversation

brawner
Copy link
Contributor

@brawner brawner commented Jul 29, 2020

This adds fault injection macros for use in rcl_action and rcl_lifecycle.

Signed-off-by: Stephen Brawner brawner@gmail.com

@brawner brawner self-assigned this Jul 29, 2020
@brawner brawner force-pushed the brawner/fault-injection-macros branch from 897314f to 1861408 Compare August 11, 2020 23:45
@brawner brawner force-pushed the brawner/fault-injection-macros branch from 1861408 to 8c51c9f Compare August 27, 2020 18:17
@brawner
Copy link
Contributor Author

brawner commented Aug 27, 2020

Testing this PR branch against current ros 2 branches (--packages-select rmw)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link

@Blast545 Blast545 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Questions about API in general, might need a follow up PR or a separated discussion

@@ -105,6 +108,8 @@ _rmw_topic_endpoint_info_copy_str(
const char * str,
rcutils_allocator_t * allocator)
{
RCUTILS_CAN_RETURN_WITH_ERROR_OF(RMW_RET_INVALID_ARGUMENT);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be able of returning BAD_ALLOC as well? I don't know why the output of rcutils_strdup is not checked there

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same with the remaining part of the API receiving an allocator

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #269 to add the new API and added a second RCUTILS_CAN_RETURN_WITH_ERROR_OF here.

@brawner brawner force-pushed the brawner/fault-injection-macros branch from 8c51c9f to 09d4797 Compare August 27, 2020 23:29
@brawner brawner changed the base branch from master to brawner/bad-alloc-ret August 27, 2020 23:32
@brawner brawner changed the base branch from brawner/bad-alloc-ret to master August 31, 2020 18:27
@brawner
Copy link
Contributor Author

brawner commented Aug 31, 2020

Rebased onto master after merging #269, retesting

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner
Copy link
Contributor Author

brawner commented Aug 31, 2020

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! Also, should it be rebased on top of #269? Either that or Github UI got confused.

Signed-off-by: Stephen Brawner <brawner@gmail.com>
Signed-off-by: Stephen Brawner <brawner@gmail.com>
Signed-off-by: Stephen Brawner <brawner@gmail.com>
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner force-pushed the brawner/fault-injection-macros branch from 724a377 to 3f8c2e4 Compare August 31, 2020 19:48
@brawner
Copy link
Contributor Author

brawner commented Aug 31, 2020

A rebase did the trick

Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go!

@brawner brawner merged commit 518716c into master Aug 31, 2020
@delete-merged-branch delete-merged-branch bot deleted the brawner/fault-injection-macros branch August 31, 2020 19:52
ahcorde pushed a commit that referenced this pull request Oct 13, 2020
* Add fault injection macros for use in other packages

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* cxx/c flags

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Address feedback

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* lint cmake

Signed-off-by: Stephen Brawner <brawner@gmail.com>
ahcorde pushed a commit that referenced this pull request Oct 13, 2020
* Add fault injection macros for use in other packages

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* cxx/c flags

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Address feedback

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* lint cmake

Signed-off-by: Stephen Brawner <brawner@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants