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
Fix the potential application crash issues #426
Conversation
rclcpp/src/rclcpp/publisher.cpp
Outdated
auto intra_process_topic_name = std::string(this->get_topic_name()) + "/_intra"; | ||
const char * topic_name = this->get_topic_name(); | ||
if (topic_name) { | ||
auto intra_process_topic_name = std::string(topic_name) + "/_intra"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tried to compile the patch locally (and ran the tests)? It look to me that the variable has a local scope and therefore isn't available below where it is being used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
badly, it becomes a local scope within if{}, and I changed it. Now all have passed the build and tests, thanks !
rclcpp/src/rclcpp/publisher.cpp
Outdated
intra_process_topic_name = std::string(topic_name) + "/_intra"; | ||
} else { | ||
throw std::runtime_error("Get a invalid null topic name"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a similar structure here as above. Check for !topic_name
and conditionally throw
, Then the else
case becomes obsolete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense, changed 😄
Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>
Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>
Builds are all green, @dirk-thomas thinks it is OK, looks OK to me. Merging. |
This way we avoid tripping a Fast-CDR check for a 0 or 1 in the bool field. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
get_topic_name
andrcl_service_get_service_name
may return NULLSigned-off-by: Ethan Gao ethan.gao@linux.intel.com