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

Port small fixes from Melodic #175

Merged
merged 3 commits into from
Oct 23, 2020
Merged

Port small fixes from Melodic #175

merged 3 commits into from
Oct 23, 2020

Conversation

artivis
Copy link
Contributor

@artivis artivis commented Feb 21, 2020

@artivis
Copy link
Contributor Author

artivis commented Feb 21, 2020 •
edited
Loading

@nuclearsandwich Writing tests to cover those specific fixes feels flaky. Those fixes do not throw or the error is caught and redirected through the LOG_ERROR macro. Furthermore the interesting bits of ClassLoader API are private which doesn't help either.

Edit: I guess one could search for the error message in stderr (e.g.::testing::internal::GetCapturedStderr()) ...

@artivis artivis changed the title Fix/port check for null Port small fixes from Melodic Feb 21, 2020
@nuclearsandwich nuclearsandwich self-assigned this Mar 5, 2020
@nuclearsandwich nuclearsandwich self-requested a review March 5, 2020 19:59
@nuclearsandwich nuclearsandwich added the ros2 Issues and PRs relating to ROS 2 label Apr 21, 2020
@kyrofa
Copy link

kyrofa commented Jun 5, 2020

Hey @nuclearsandwich, does this one look okay?

@artivis @clalancette
Check for NULL in XMLNode::Value
390a196 
Signed-off-by: artivis <jeremie.deray@canonical.com>
@artivis @clalancette
Check for NULL in XMLElement::GetText
e6d9281 
Signed-off-by: artivis <jeremie.deray@canonical.com>
@artivis @clalancette
Check for NULL in XMLElement::Attribute
532ad9b 
std::string my_string(NULL) is UB
  Fix ros#160

Signed-off-by: artivis <jeremie.deray@canonical.com>
@clalancette
Copy link

I've rebased this onto the latest. The code looks good to me, so I'll approve and run CI on it.

@clalancette
Copy link

CI:

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

@clalancette clalancette merged commit 90c4fb4 into ros:ros2 Oct 23, 2020
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

@nuclearsandwich nuclearsandwich Awaiting requested review from nuclearsandwich

Assignees

@nuclearsandwich nuclearsandwich

Labels
ros2 Issues and PRs relating to ROS 2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@artivis @kyrofa @clalancette @nuclearsandwich