Skip to content

Update init/shutdown API documentation.#243

Merged
hidmic merged 2 commits intomasterfrom
hidmic/update-init-shutdown-docs
Jun 29, 2020
Merged

Update init/shutdown API documentation.#243
hidmic merged 2 commits intomasterfrom
hidmic/update-init-shutdown-docs

Conversation

@hidmic
Copy link
Copy Markdown

@hidmic hidmic commented Jun 23, 2020

Constrain behavior for known context states. Alternative to #242.

Constrain behavior for known context states.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
* guard conditions, and is also required to properly call `rmw_shutdown()`.
*
* \pre The given context must be zero initialized.
*
Copy link
Copy Markdown
Member

@ivanpauno ivanpauno Jun 23, 2020

Choose a reason for hiding this comment

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

is it possible to ensure that the context will remain zero initialized if rmw_init failed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll assume you mean rmw_init(). If we can provide an adequate return code, we can rollback what we've done and reset the content of the struct. Note this talks about the rmw_context_t struct and not about any other global state that an rmw implementation may initialize as a side effect. We can't make promises there.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If it doesn't add much burden to the implementation, I would try to always enforce a "strong failure guarantee" (equivalent to the C++ strong exception safety guarantee), more than only a "basic failure" guarantee).

I'll assume you mean rmw_init()

Yes, edited.

Note this talks about the rmw_context_t struct and not about any other global state that an rmw implementation may initialize as a side effect

yes, there might be visible side effects that we can't control.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If it doesn't add much burden to the implementation, I would try to always enforce a "strong failure guarantee"

Yeah, I know where you're going. Because I don't know for sure what is (or will be) out there, I try not to commit to more than it is strictly necessary for it not to be imprecise. In this specific case, I'd rather not make strong guarantees about anything but the struct just yet. @wjwwood for feedback.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't mind one way or the other. We don't usually reset structures on failure, but we do things like reclaim resources that would otherwise be leaked, which is a little more important in my opinion. The user could always re-zero init the structure if it failed. I think the only important guarantee is that no resources are leaked, so it is not needed to call shutdown or fini on it and it's say to overwrite it.

@hidmic hidmic requested a review from brawner June 23, 2020 17:06
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic
Copy link
Copy Markdown
Author

hidmic commented Jun 29, 2020

Alright, going in!

@hidmic hidmic merged commit 4a1914a into master Jun 29, 2020
@delete-merged-branch delete-merged-branch bot deleted the hidmic/update-init-shutdown-docs branch June 29, 2020 17:14
ahcorde pushed a commit that referenced this pull request Oct 13, 2020
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
ahcorde pushed a commit that referenced this pull request Oct 13, 2020
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.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.

5 participants