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 rcutils_string_array_resize function #247

Merged
merged 19 commits into from
Jun 8, 2020
Merged

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented May 13, 2020

This change adds a new function to safely modify the size of an array which is already allocated, preserving existing entries where possible. It combines the existing behaviors in rcutils_string_array_init and rcutils_string_array_fini.

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

This change adds a new function to modify the size of an array which is
already allocated, preserving existing entries where possible.

Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay added enhancement New feature or request in review Waiting for review (Kanban column) labels May 13, 2020
@cottsay cottsay self-assigned this May 13, 2020
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Check the allocator

cottsay and others added 2 commits May 14, 2020 10:21
Signed-off-by: Scott K Logan <logans@cottsay.net>

Co-authored-by: Alejandro Hernández Cordero <alejandro@openrobotics.org>
Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay requested a review from ahcorde May 14, 2020 22:02
cottsay added 2 commits May 15, 2020 13:22
Signed-off-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Scott K Logan <logans@cottsay.net>
cottsay added 2 commits May 19, 2020 17:33
Signed-off-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay requested a review from Karsten1987 May 20, 2020 00:47
cottsay added 3 commits May 20, 2020 11:02
Signed-off-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Scott K Logan <logans@cottsay.net>
cottsay and others added 4 commits June 1, 2020 14:46
Signed-off-by: Scott K Logan <logans@cottsay.net>

Co-authored-by: Karsten Knese <Karsten1987@users.noreply.github.com>
This means there is no chance that the instance will ever get
"partially" updated, even when there are problems reclaiming resources.

Signed-off-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay force-pushed the resize_string_array branch from c17656d to b808905 Compare June 1, 2020 21:58
Signed-off-by: Scott K Logan <logans@cottsay.net>
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm

@Karsten1987
Copy link
Contributor

you might want to trigger another round of CI though.

@cottsay
Copy link
Member Author

cottsay commented Jun 2, 2020

you might want to trigger another round of CI though.

I'm no longer motivated to make this happen before Foxy, so I'll trigger some fresh CI next week and we can consider merging then.

Thanks everyone for the reviews.

Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay force-pushed the resize_string_array branch from 39c0356 to 9ceb82b Compare June 8, 2020 21:23
@cottsay
Copy link
Member Author

cottsay commented Jun 8, 2020

As promised, fresh CI (with coverage):

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

cottsay added 3 commits June 8, 2020 14:44
Signed-off-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Scott K Logan <logans@cottsay.net>
@cottsay cottsay merged commit d620bdd into master Jun 8, 2020
@delete-merged-branch delete-merged-branch bot deleted the resize_string_array branch June 8, 2020 22:46
@nuclearsandwich
Copy link
Member

nuclearsandwich commented Jun 9, 2020

I think this pull request has introduced the regression ros2/rmw#234

I have run Build Status which is a mainline CI job testing rmw and the same job Build Status but with rcutils pinned back to 4157542 to demonstrate the difference although I haven't yet determined why.

Edit: I accidentally a whole build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request in review Waiting for review (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants