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

More cleanups around the code. #109

Merged
merged 5 commits into from
Feb 16, 2024
Merged

More cleanups around the code. #109

merged 5 commits into from
Feb 16, 2024

Conversation

clalancette
Copy link
Collaborator

As I was creating the design document, I noticed a few things around the code that could be cleaned up and made faster. This PR:

  1. Switches to using a keyexpression slice for creating keyexpressions. That way, we don't have to copy a string just to remove the final '/'.
  2. Makes sure that graph_sub_data_handler drops the keyexpression on all paths. In particular, on error paths we would forget to drop the keyexpression, so just use a make_scope_exit.
  3. Remove another unnecessary comment that has to do with porting to zenoh-pico.
  4. Remove an unnecessary use of malloc/memcpy/free when creating a map to use as an attachment for service calls. Since z_bytes_map_insert_by_copy will do a copy anyway, just pass in the array.
  5. When doing rmw_destroy_service, call z_undeclare_queryable to drop the queryable. I believe that this is functionally equivalent to z_drop, but I also think it is more self-documenting to use z_undeclare_queryable here.

That way we don't have to do an allocation here.

Signed-off-by: Chris Lalancette <[email protected]>
Get rid of some unnecessary operations, and be sure
to always drop the keyexpr, even on error.

Signed-off-by: Chris Lalancette <[email protected]>
Signed-off-by: Chris Lalancette <[email protected]>
It's just the counterpart to z_declare_queryable.

Signed-off-by: Chris Lalancette <[email protected]>
Copy link
Member

@Yadunund Yadunund left a comment

Choose a reason for hiding this comment

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

Thanks for the additional cleanup!

@Yadunund Yadunund merged commit 2d4bac7 into rolling Feb 16, 2024
5 checks passed
@delete-merged-branch delete-merged-branch bot deleted the clalancette/more-cleanups branch February 16, 2024 10:18
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.

2 participants