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.

Sign up for GitHub

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

Jump to bottom

fix mismatched issue if using zero_allocate #1995

Merged
merged 3 commits into from
Sep 15, 2022
Merged

fix mismatched issue if using zero_allocate #1995

merged 3 commits into from
Sep 15, 2022

Conversation

iuhilnehc-ynos
Copy link
Collaborator

@iuhilnehc-ynos iuhilnehc-ynos commented Aug 18, 2022 •
edited
Loading

to fix #1996

Signed-off-by: Chen Lihui lihui.chen@sony.com

@iuhilnehc-ynos iuhilnehc-ynos mentioned this pull request Aug 18, 2022
Closed
fujitatomoya
fujitatomoya approved these changes Aug 18, 2022
View reviewed changes
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

@iuhilnehc-ynos
Copy link
Collaborator Author

iuhilnehc-ynos commented Aug 19, 2022 •
edited
Loading

Notice that SubscriptionOptions use allocator<char> instead of allocator<void> after 0659d82

image

image

The allocator in a subscription(rcl) will not use void version but non-void version while creating a subscription(rclcpp), then the mismatched issue happened.

We won't roll back the void version, right? It seems we need a zero_allocate for the c++ version.

@iuhilnehc-ynos iuhilnehc-ynos changed the title fix mismatched issue if uzing zero_allocated fix mismatched issue if using zero_allocate Aug 19, 2022
@fujitatomoya
Copy link
Collaborator

related to #1657

@fujitatomoya fujitatomoya mentioned this pull request Sep 13, 2022
Open
Chen Lihui added 3 commits September 14, 2022 15:23
@clalancette
fix mismatched issue if uzing zero_allocated
13b0ad2 
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@clalancette
call memset
eb05271 
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@clalancette
add test
a9f3352 
Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@clalancette
Copy link
Contributor

CI:

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

@clalancette
Copy link
Contributor

The Rpr job is expected to fail right now, as we are in the middle of a large rebuild. Assuming CI comes back clean, I'll rerun Rpr later on (probably tomorrow), when the rebuild should be complete.

@clalancette
Copy link
Contributor

@ros-pull-request-builder retest this please

@iuhilnehc-ynos
Copy link
Collaborator Author

iuhilnehc-ynos commented Sep 15, 2022 •
edited
Loading

@ros-pull-request-builder retest this please

projectroot.test.rclcpp.test_time_source
rclcpp.TestTimeSource.callbacks
rclcpp.TestTimeSource.callback_handler_erasure

It seems that Rpr failure is not related to current PR.
I am afraid that updating from ==(PR, reason) to >=(PR) is not the intention in the original PR.

@clalancette
Copy link
Contributor

It seems that Rpr failure is not related to current PR.

Yeah, agreed. I'm going to go ahead and merge this one in now that it is all green. Thanks @iuhilnehc-ynos !

@clalancette clalancette merged commit 9784391 into ros2:rolling Sep 15, 2022
@cwecht
Copy link
Contributor

cwecht commented Oct 15, 2022

@clalancette are considering this patch for backporting to humble? I'd very much appreciate it, as this patch would fix allocator support in humble.

@fujitatomoya
Copy link
Collaborator

@Mergifyio backport humble

@mergify
Copy link

mergify bot commented Oct 16, 2022

backport humble

✅ Backports have been created

@mergify mergify bot mentioned this pull request Oct 16, 2022
Merged
mergify bot pushed a commit that referenced this pull request Oct 16, 2022
@mergify
fix mismatched issue if using zero_allocate (#1995)
193eb93 
* fix mismatched issue if uzing zero_allocated

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
(cherry picked from commit 9784391)
fujitatomoya pushed a commit that referenced this pull request Oct 17, 2022
@mergify
fix mismatched issue if using zero_allocate (#1995) (#2026)
4fa3489 
* fix mismatched issue if uzing zero_allocated

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
(cherry picked from commit 9784391)

Co-authored-by: Chen Lihui <lihui.chen@sony.com>
@fujitatomoya
Copy link
Collaborator

@cwecht backport humble is done.

@cwecht
Copy link
Contributor

cwecht commented Oct 17, 2022

@fujitatomoya thank you very much!

@iuhilnehc-ynos iuhilnehc-ynos mentioned this pull request Feb 24, 2023
Merged
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Apr 29, 2023
fix mismatched issue if using zero_allocate (ros2#1995)
50a2bb6 
* fix mismatched issue if uzing zero_allocated

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request Apr 29, 2023
fix mismatched issue if using zero_allocate (ros2#1995)
3f4ce2f 
* fix mismatched issue if uzing zero_allocated

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request May 3, 2023
fix mismatched issue if using zero_allocate (ros2#1995)
76cfcb6 
* fix mismatched issue if uzing zero_allocated

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
alsora pushed a commit to irobot-ros/rclcpp that referenced this pull request May 3, 2023
fix mismatched issue if using zero_allocate (ros2#1995)
51b032e 
* fix mismatched issue if uzing zero_allocated

Signed-off-by: Chen Lihui <lihui.chen@sony.com>
@iuhilnehc-ynos iuhilnehc-ynos mentioned this pull request Jun 21, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

fix mismatched about calling calloc and delete
4 participants
@iuhilnehc-ynos @fujitatomoya @clalancette @cwecht