utils: fix integer divide-by-zero on Windows x64 in bytes_to_human_readable_size#11727
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe loop/index variables in Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/flb_utils.c (1)
815-827:⚠️ Potential issue | 🟠 MajorPrevent the divisor from overflowing at EiB.
Changing
utouint64_tfixes the Windows x64 GiB case, but the loop can still wraputo zero whenbytes >= 1024^6because line 826 attempts1024^7, then the next iteration divides by zero. Track the current unit divisor instead of the next threshold.🐛 Proposed fix
- uint64_t i; - uint64_t u = 1024; + uint64_t i; + uint64_t u = 1; @@ - for (i = 0; __units[i] != NULL; i++) { - if ((bytes / u) == 0) { - break; - } + for (i = 0; __units[i + 1] != NULL && (bytes / u) >= 1024; i++) { u *= 1024; } if (!i) { snprintf(out_buf, size, "%lu%s", (long unsigned int) bytes, __units[0]); } else { - float fsize = (float) ((double) bytes / (u / 1024)); + float fsize = (float) ((double) bytes / u); snprintf(out_buf, size, "%.1f%s", fsize, __units[i]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/flb_utils.c` around lines 815 - 827, The loop uses u as the next-threshold divisor and multiplies it until overflow (causing divide-by-zero when bytes >= 1024^6); update the logic in the function around the __units loop so you track the current unit divisor (e.g., a variable like div starting at 1) and advance units while bytes / div >= 1024 (and the next unit exists), instead of pre-multiplying u to the next threshold; also guard the multiplication by checking for potential overflow (or compare bytes/div to 1024) before multiplying u so you never wrap u to zero — adjust references to u and the unit selection accordingly in this loop that iterates over __units.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/flb_utils.c`:
- Around line 815-827: The loop uses u as the next-threshold divisor and
multiplies it until overflow (causing divide-by-zero when bytes >= 1024^6);
update the logic in the function around the __units loop so you track the
current unit divisor (e.g., a variable like div starting at 1) and advance units
while bytes / div >= 1024 (and the next unit exists), instead of pre-multiplying
u to the next threshold; also guard the multiplication by checking for potential
overflow (or compare bytes/div to 1024) before multiplying u so you never wrap u
to zero — adjust references to u and the unit selection accordingly in this loop
that iterates over __units.
…adable_size On Windows x64, `unsigned long` is 32 bits (LLP64 data model), unlike Linux x64 where it is 64 bits (LP64). In flb_utils_bytes_to_human_readable_size(), the divisor `u` starts at 1024 and is multiplied by 1024 each iteration. After three iterations (reaching the GB unit), the next multiplication overflows a 32-bit unsigned long to zero, causing an integer divide-by-zero crash (exception code 0xc0000094). This crash is triggered whenever the storage metrics timer (every 5 seconds) calls the formatter with a buffer size exceeding ~1 GB, which is common with high-volume inputs using filesystem-backed storage. Change `unsigned long` to `uint64_t` for both the loop counter and the divisor, guaranteeing 64-bit arithmetic on all platforms. Signed-off-by: Amit Shinde <ashinde@databahn.ai> Made-with: Cursor
5c984ac to
2caa52d
Compare
cosmo0920
left a comment
There was a problem hiding this comment.
Yes, this is a pitfall of Windows LLP64.
So, we really need for this fix.
Summary
Fix an
INTEGER_DIVIDE_BY_ZEROcrash (exception0xC0000094) on Windows x64 inflb_utils_bytes_to_human_readable_size().Root Cause
On Windows x64, the LLP64 data model makes
unsigned long32 bits (unlike LP64 onLinux where it is 64 bits). The divisor variable
ustarts at 1024 and is multipliedby 1024 each loop iteration. After three iterations (reaching the GB unit boundary),
the next multiplication (
1024^4 = 1,099,511,627,776) overflows a 32-bit unsignedlong to zero, causing a divide-by-zero crash on the next
bytes / uoperation.This is triggered every 5 seconds by the storage metrics timer
(
cb_storage_metrics_collectinflb_storage.c) whenever any input buffer exceeds ~1 GB,which is common with high-volume inputs using filesystem-backed storage.
Fix
Change
unsigned longtouint64_tfor both the loop counteriand divisoru,guaranteeing 64-bit arithmetic on all platforms.
Testing
winevtloginput producing >1 GB of buffered data.Before the fix, Fluent Bit crashes every ~5 seconds at the metrics timer. After the
fix, the formatter correctly displays sizes in GB/TB without overflow.
Signed-off-by: Amit Shinde ashinde@databahn.ai
Summary by CodeRabbit