Skip to content

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 [email protected]

@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

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 <[email protected]>
Signed-off-by: Stephen Brawner <[email protected]>
Signed-off-by: Stephen Brawner <[email protected]>
@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 <[email protected]>

* cxx/c flags

Signed-off-by: Stephen Brawner <[email protected]>

* Address feedback

Signed-off-by: Stephen Brawner <[email protected]>

* lint cmake

Signed-off-by: Stephen Brawner <[email protected]>
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 <[email protected]>

* cxx/c flags

Signed-off-by: Stephen Brawner <[email protected]>

* Address feedback

Signed-off-by: Stephen Brawner <[email protected]>

* lint cmake

Signed-off-by: Stephen Brawner <[email protected]>
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.

4 participants