W-M-R opened a new pull request, #18321: URL: https://github.com/apache/nuttx/pull/18321
*Note: Please adhere to [Contributing Guidelines](https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md).* ## Summary This patch series fixes critical bugs in the file input stream reading logic that caused data corruption and resolves a static analysis warning. Problem 1 - Data corruption in loop reads (549c6f1): The original implementation had a fundamental flaw: when file_read() returned fewer bytes than requested, the loop would retry reading without advancing the buffer pointer or adjusting the remaining length. This caused subsequent reads to overwrite previously read data, resulting in data loss and corruption. Problem 2 - Impossible condition (b7339ba): Static analysis detected that the loop condition while (left >= 0) was meaningless because left is size_t (unsigned), which can never be less than 0. The condition should check left > 0 to properly terminate when all data is read. Changes made: Fixed buffer advancement: Added proper pointer arithmetic (buf += ret) to advance the buffer position after each successful read Fixed remaining length tracking: Properly decrement left after each read to track remaining bytes Improved return value: Return total bytes read (len - left) instead of just the last read result, handling partial reads correctly Fixed loop termination: Changed while (left >= 0) to while (left > 0) to avoid infinite loop and resolve static analysis warning Better error handling: Distinguish between EINTR (retry), errors (break), and EOF (break) Fixed typo: Corrected struct name from lib_fileinstream_s to lib_filesistream_s ## Impact Functionality: Critical fix: Prevents data corruption when reading large files requiring multiple file_read() calls Eliminates infinite loop possibility from incorrect loop condition Proper handling of partial reads (common with pipes, network streams, or interrupted system calls) Accurate byte count tracking through self->nget Correctness: Resolves static analysis warning about impossible unsigned comparison Proper POSIX-compliant behavior for partial reads Correct return value semantics (total bytes read vs. last operation result) Affected scenarios: Reading files larger than internal buffer size Reading from slow devices (pipes, sockets, serial ports) Systems with frequent signal interruptions (EINTR) Any multi-iteration read operations ## Testing Test scenarios: Large file reads Created test files > 64KB requiring multiple read iterations Verified data integrity by checksum comparison Confirmed buffer properly advances without overwriting ✅ No data corruption detected Partial read simulation Mocked file_read() to return partial chunks (e.g., 100 bytes at a time) Verified loop correctly accumulates all data Confirmed nget counter accurately tracks total bytes ✅ All data correctly assembled EINTR handling Simulated signal interruptions during reads Verified retry logic without advancing buffer Confirmed no data loss on interrupted reads ✅ Proper retry without corruption EOF and error conditions Tested premature EOF (file shorter than requested) Verified correct return value (partial read count) Tested read errors (permission denied, bad fd) ✅ Proper error propagation Static analysis validation Ran static analyzers (cppcheck, clang-tidy) on modified code ✅ No warnings about impossible conditions ✅ No warnings about unsigned comparison with 0 Before fix: Reading 1000 bytes with file_read() returning 100 bytes each iteration would result in only 100 bytes in buffer (overwritten 9 times) After fix: Reading 1000 bytes with file_read() returning 100 bytes each iteration correctly produces 1000 bytes in buffer -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
