support 64 bit filesizes when calling ACE_OS::fstat() and _FILE_OFFSE… #2254
Conversation
WalkthroughThe changes modify the Changes
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ACE/ace/OS_NS_sys_stat.inl (1)
53-60: Consider using constants for file mode composition.The field assignments look good, but the file mode composition could be more maintainable using predefined constants.
Consider this more readable approach:
- stp->st_mode = S_IXOTH | S_IROTH | - (fdata.dwFileAttributes & FILE_ATTRIBUTE_READONLY ? 0 : S_IWOTH) | - (fdata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY ? S_IFDIR : S_IFREG); + const mode_t BASE_MODE = S_IXOTH | S_IROTH; + const mode_t WRITE_MODE = (fdata.dwFileAttributes & FILE_ATTRIBUTE_READONLY) ? 0 : S_IWOTH; + const mode_t TYPE_MODE = (fdata.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) ? S_IFDIR : S_IFREG; + stp->st_mode = BASE_MODE | WRITE_MODE | TYPE_MODE;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ACE/ace/OS_NS_sys_stat.inl(1 hunks)
🔇 Additional comments (3)
ACE/ace/OS_NS_sys_stat.inl (3)
39-43: LGTM! Good safety check for large file sizes.The condition prevents silent truncation of large file sizes by explicitly checking if the high part is non-zero and if the target type can't hold the full size. Setting errno to EINVAL is appropriate in this case.
45-52: LGTM! Proper implementation of 64-bit file size support.The code correctly uses ULARGE_INTEGER for 64-bit support while maintaining backward compatibility. This implementation aligns perfectly with the PR's objective of supporting large file sizes on Win32.
39-61: Verify handling of edge cases.Please verify the implementation handles these edge cases correctly:
- Special files (pipes, devices, etc.)
- File sizes exactly at 4GB (32-bit boundary)
- File sizes exactly at 16EB (64-bit boundary)
✅ Verification successful
Implementation correctly handles supported cases within Windows limitations
The current implementation is appropriate because:
- File size handling is robust with proper 32/64-bit support and overflow checks
- Windows-specific file types (regular files and directories) are correctly mapped
- Unsupported POSIX file types (pipes, devices, etc.) are intentionally not handled as Windows lacks direct equivalents
- The implementation safely returns errors rather than attempting unsafe mappings
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for test cases covering edge cases in fstat implementation # Look for test files echo "Searching for fstat test files..." fd -e cpp -e h -e inl "test.*fstat" # Look for specific test patterns echo "Searching for edge case tests..." rg -l "fstat.*4GB|fstat.*pipe|fstat.*device" # Look for existing implementations handling special files echo "Searching for special file handling..." ast-grep --pattern 'if \($FILE_ATTRIBUTE_DEVICE\) { $$$ }'Length of output: 557
Script:
#!/bin/bash # Search for file size and attribute handling tests echo "Searching for size-related tests..." rg -l "FileSizeHigh|FileSizeLow|_FILE_OFFSET_BITS.*64" --type cpp --type h echo "Searching for Windows file attribute handling..." rg "FILE_ATTRIBUTE_" -A 3 -B 3 echo "Searching for special file type handling..." rg "S_IF(CHR|BLK|FIFO|SOCK)" -A 2 -B 2Length of output: 6662
* ACE/ace/OS_NS_sys_stat.inl:
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
…T_BITS==64 on Win32 systems
Summary by CodeRabbit