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

Compile flag fixes #114

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/cmdline_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ bool rcutils_cli_option_exist(char ** begin, char ** end, const char * option)
char * rcutils_cli_get_option(char ** begin, char ** end, const char * option)
{
size_t idx = 0;
size_t end_idx = end - begin;
size_t end_idx = (size_t)(end - begin);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking around a bit, I think the basic problem here is that taking the difference between two pointers should be represented by ptrdiff_t, not size_t (in theory, the difference can be negative: https://bytes.com/topic/c/answers/460624-subtracting-pointers-type-use-store-result). While changing to this, you'll probably also have to change the type of idx to match.

for (; idx < end_idx; ++idx) {
if (strncmp(begin[idx], option, strlen(option)) == 0) {
break;
Expand Down
2 changes: 1 addition & 1 deletion src/error_handling.c
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ format_error_string(void)
if (!__rcutils_error_is_set(__rcutils_error_state)) {
return;
}
size_t bytes_it_would_have_written = snprintf(
size_t bytes_it_would_have_written = (size_t)snprintf(
Copy link
Contributor

Choose a reason for hiding this comment

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

There's actually a few problems here. snprintf is defined to return an integer, because the return value can be negative on failure. So the return value should be stored in an integer. After that, bytes_it_would_have_written should be checked for negative; if it failed, we definitely don't want to continue on and pass bogus sizes around as unsigned. Finally, we would need a cast from int to size_t while passing bytes_it_would_have_written into allocator.allocate() and the lower-down snprintf to avoid the warnings.

NULL, 0,
__error_format_string,
__rcutils_error_state->message,
Expand Down
2 changes: 1 addition & 1 deletion src/format_string.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ rcutils_format_string_limit(
va_list args2;
va_copy(args2, args1);
// first calculate the output string
size_t bytes_to_be_written = rcutils_vsnprintf(NULL, 0, format_string, args1);
size_t bytes_to_be_written = (size_t)rcutils_vsnprintf(NULL, 0, format_string, args1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Like in error_handling.c, the return value of rcutils_vsnprintf is actually int, not size_t, since it can fail. The same comments apply about storing in an int, checking for negative, and then casting where it is used.

va_end(args1);
// allocate space for the return string
if (bytes_to_be_written + 1 > limit) {
Expand Down
4 changes: 2 additions & 2 deletions src/logging.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ rcutils_logging_severity_level_from_string(
return RCUTILS_RET_BAD_ALLOC;
}
for (int i = 0; severity_string_upper[i]; ++i) {
severity_string_upper[i] = toupper(severity_string_upper[i]);
severity_string_upper[i] = (char)toupper(severity_string_upper[i]);
}

// Determine the severity value matching the severity name.
Expand Down Expand Up @@ -514,7 +514,7 @@ void rcutils_logging_console_output_handler(
}
if ((size_t)written >= sizeof(static_message_buffer)) {
// write was incomplete, allocate necessary memory dynamically
size_t message_buffer_size = written + 1;
size_t message_buffer_size = (size_t)written + 1U;
void * dynamic_message_buffer = g_rcutils_logging_allocator.allocate(
message_buffer_size, g_rcutils_logging_allocator.state);
if (NULL == dynamic_message_buffer) {
Expand Down
4 changes: 2 additions & 2 deletions src/repl_str.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,11 @@ rcutils_repl_str(
}
}

pos_cache[count-1] = pstr2 - str;
pos_cache[count-1] = (uintptr_t)(pstr2 - str);
pstr = pstr2 + fromlen;
}

orglen = pstr - str + strlen(pstr);
orglen = (size_t)(pstr - str) + strlen(pstr);

/* Allocate memory for the post-replacement string. */
if (count > 0) {
Expand Down
2 changes: 1 addition & 1 deletion src/time.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ rcutils_time_point_value_as_seconds_string(
}
// best to abs it to avoid issues with negative values in C89, see:
// https://stackoverflow.com/a/3604984/671658
uint64_t abs_time_point = llabs(*time_point);
uint64_t abs_time_point = (uint64_t)llabs(*time_point);
// break into two parts to avoid floating point error
uint64_t seconds = abs_time_point / (1000 * 1000 * 1000);
uint64_t nanoseconds = abs_time_point % (1000 * 1000 * 1000);
Expand Down