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

Avoid unused return value warning #168

Merged
merged 1 commit into from
Dec 4, 2015
Merged

Avoid unused return value warning #168

merged 1 commit into from
Dec 4, 2015

Conversation

esteve
Copy link
Member

@esteve esteve commented Dec 4, 2015

This makes sure that the returned value of strerror_r is used.

@esteve esteve added the in progress Actively being worked on (Kanban column) label Dec 4, 2015
@esteve esteve added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Dec 4, 2015
@esteve esteve self-assigned this Dec 4, 2015
@dirk-thomas
Copy link
Member

The whole change is in a #ifndef _WIN32 block. Why do we need to run three Windows CI jobs?

Also #140 (comment) still applies.

@esteve
Copy link
Member Author

esteve commented Dec 4, 2015

The warning is fixed in Linux:

http://ci.ros2.org/view/packaging/job/packaging_linux/53/warnings17Result/fixed/

I don't know if the warning existed in OSX, the packaging job does not report anything, but the ifdef is needed, otherwise it wouldn't compile.

char * msg = strerror_r(errno, error_string, error_length);
if (msg != error_string) {
strncpy(error_string, msg, sizeof(error_string));
}
Copy link
Member

Choose a reason for hiding this comment

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

If msg would be a string with 1024 or more characters and different from error_string the strncpy will make error_string a non-null terminated string which will likely make the subsequent throw in line 135 explode?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@esteve esteve force-pushed the avoid_compiler_warning branch from b72c43a to a57c9b9 Compare December 4, 2015 18:26
@dirk-thomas
Copy link
Member

Can you please not trigger Windows jobs for this.

#ifdef _GNU_SOURCE
char * msg = strerror_r(errno, error_string, error_length);
if (msg != error_string) {
strncpy(error_string, msg, sizeof(error_string));
Copy link
Member

Choose a reason for hiding this comment

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

The variable error_length already contains the value of sizeof(error_string).

Same next line.

@esteve
Copy link
Member Author

esteve commented Dec 4, 2015

@dirk-thomas duly noted. There weren't any jobs in the queue and nobody was using the Windows machine, so there was no harm in running the jobs anyway :-)

@esteve esteve force-pushed the avoid_compiler_warning branch from a57c9b9 to f0fd292 Compare December 4, 2015 19:09
@esteve
Copy link
Member Author

esteve commented Dec 4, 2015

@esteve
Copy link
Member Author

esteve commented Dec 4, 2015

This should be ready for merging, the packaging jobs don't report the warning any more.

@wjwwood
Copy link
Member

wjwwood commented Dec 4, 2015

lgtm

esteve added a commit that referenced this pull request Dec 4, 2015
Avoid unused return value warning
@esteve esteve merged commit 67dcd9f into master Dec 4, 2015
@esteve esteve removed the in review Waiting for review (Kanban column) label Dec 4, 2015
@esteve esteve deleted the avoid_compiler_warning branch December 4, 2015 20:27
@dirk-thomas
Copy link
Member

+1

nnmm pushed a commit to ApexAI/rclcpp that referenced this pull request Jul 9, 2022
remove obsolete INDENT-OFF usage
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
* adopt to changes in rclcpp::subscription

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* use init/fini function from introspection_ts

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* fix line length

Signed-off-by: Karsten Knese <karsten@openrobotics.org>
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.

3 participants