Skip to content

[libc] Fix incorrect unsigned comparison #135595

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

Merged
merged 3 commits into from
Apr 17, 2025
Merged

[libc] Fix incorrect unsigned comparison #135595

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
806f26b
fix
yingcong-wu Apr 14, 2025
1c6c1b5
remove redundent if
yingcong-wu Apr 15, 2025
38b5517
trigger CI
yingcong-wu Apr 16, 2025
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
16 changes: 7 additions & 9 deletions libc/src/stdio/printf_core/float_dec_converter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,12 @@ template <WriteMode write_mode> class FloatWriter {
if (total_digits_written < digits_before_decimal &&
total_digits_written + buffered_digits >= digits_before_decimal &&
has_decimal_point) {
// digits_to_write > 0 guaranteed by outer if
size_t digits_to_write = digits_before_decimal - total_digits_written;
if (digits_to_write > 0) {
// Write the digits before the decimal point.
RET_IF_RESULT_NEGATIVE(writer->write({block_buffer, digits_to_write}));
}
// Write the digits before the decimal point.
RET_IF_RESULT_NEGATIVE(writer->write({block_buffer, digits_to_write}));
RET_IF_RESULT_NEGATIVE(writer->write(DECIMAL_POINT));
if (buffered_digits - digits_to_write > 0) {
if (buffered_digits > digits_to_write) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The outer if condition guarantees that buffered_digits - digits_to_write >= 0. So underflow does not occur here. However, the proposed change enhances clarity and readability.

Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, the if (digits_to_write > 0) statement on line 190 is redundant, as the outer if already guarantees the condition to be true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that the if in line 190 is redundant and that is quite clear, but I don't think it is straightforward that the outer if also guarantees buffered_digits - digits_to_write >= 0, not to me anyway. I will remove the redundant if in line 190.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that the underflow shouldn't be possible, but the proposed change improves clarity.

// Write the digits after the decimal point.
RET_IF_RESULT_NEGATIVE(
writer->write({block_buffer + digits_to_write,
Expand All @@ -217,12 +216,11 @@ template <WriteMode write_mode> class FloatWriter {
total_digits_written + BLOCK_SIZE * max_block_count >=
digits_before_decimal &&
has_decimal_point) {
// digits_to_write > 0 guaranteed by outer if
size_t digits_to_write = digits_before_decimal - total_digits_written;
if (digits_to_write > 0) {
RET_IF_RESULT_NEGATIVE(writer->write(MAX_BLOCK_DIGIT, digits_to_write));
}
RET_IF_RESULT_NEGATIVE(writer->write(MAX_BLOCK_DIGIT, digits_to_write));
RET_IF_RESULT_NEGATIVE(writer->write(DECIMAL_POINT));
if ((BLOCK_SIZE * max_block_count) - digits_to_write > 0) {
if ((BLOCK_SIZE * max_block_count) > digits_to_write) {
RET_IF_RESULT_NEGATIVE(writer->write(
MAX_BLOCK_DIGIT, (BLOCK_SIZE * max_block_count) - digits_to_write));
}
Expand Down
Loading