Fix getting file size for NativeFileInfo #2996
Conversation
|
Warning Rate limit exceeded@josesimoes has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 59 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent changes enhance the file information retrieval mechanisms across multiple classes in the system, specifically targeting the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant System
participant FileSystem
User->>System: Request file length
System->>FileSystem: Call GetFileInfo
FileSystem-->>System: Return file size and attributes
System-->>User: Provide file length
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
josesimoes
added
Area: Common libs
![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
Outside diff range, codebase verification and nitpick comments (1)
targets/ChibiOS/_littlefs/littlefs_FS_Driver.cpp (1)
1217-1217: Formatting improvement.The addition of a newline character at the end of the
g_LITTLEFS_FILE_SYSTEM_DriverInterfacedeclaration improves the code's formatting.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_NativeFindFile.cpp (1 hunks)
- targets/ChibiOS/_FatFs/fatfs_FS_Driver.cpp (1 hunks)
- targets/ChibiOS/_littlefs/littlefs_FS_Driver.cpp (2 hunks)
- targets/ESP32/_FatFs/fatfs_FS_Driver.cpp (1 hunks)
- targets/ESP32/_IDF/esp32/app_main.c (1 hunks)
- targets/ESP32/_littlefs/littlefs_FS_Driver.cpp (1 hunks)
- targets/netcore/littlefs/littlefs_FS_Driver.cpp (1 hunks)
Additional comments not posted (11)
targets/ESP32/_IDF/esp32/app_main.c (2)
64-64: Verify the rationale behind changing core affinity.The
receiver_taskis now pinned to core 1. Ensure this change optimizes the task scheduling and does not introduce any race conditions.
67-67: Verify the rationale behind changing core affinity.The
main_taskis now pinned to core 0. Ensure this change optimizes the task scheduling and does not introduce any race conditions.src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_NativeFindFile.cpp (2)
28-29: Enhance robustness by zero-initializing structures.Zero-initializing the
fileInfostructure prevents the use of uninitialized data, enhancing robustness.
36-36: Ensure consistency with type definition lookup.Updating the type definition lookup to
NativeFileInfoensures consistency with the intended representation.targets/ESP32/_FatFs/fatfs_FS_Driver.cpp (3)
645-649: Differentiate file attributes for directories.Setting
fileInfo->AttributestoFileAttributes::FileAttributes_Directoryfor directories enhances the accuracy of file metadata.
650-653: Differentiate file attributes for archives.Setting
fileInfo->AttributestoFileAttributes::FileAttributes_Archivefor non-directory files enhances the accuracy of file metadata.
659-659: Store file sizes explicitly.Storing the file size in
fileInfo->Sizeimproves the functionality of theGetFileInfomethod by providing more detailed metadata.targets/ChibiOS/_FatFs/fatfs_FS_Driver.cpp (1)
747-749: LGTM! The addition correctly sets the file size.The new line of code enhances the functionality by ensuring the file size is included in the information returned to the caller.
targets/ESP32/_littlefs/littlefs_FS_Driver.cpp (1)
702-704: LGTM! The addition correctly sets the file size.The new line of code enhances the functionality by ensuring the file size is included in the information returned to the caller.
targets/netcore/littlefs/littlefs_FS_Driver.cpp (1)
705-706: LGTM! The addition correctly sets the file size.The new line of code enhances the functionality by ensuring the file size is included in the information returned to the caller.
targets/ChibiOS/_littlefs/littlefs_FS_Driver.cpp (1)
736-737: LGTM! The file size assignment is a significant enhancement.The addition of
fileInfo->Size = info.size;ensures that the file size is now populated in the returnedfileInfostructure, allowing callers to access the file size directly.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_NativeFindFile.cpp (1 hunks)
- targets/ChibiOS/_FatFs/fatfs_FS_Driver.cpp (1 hunks)
- targets/ChibiOS/_littlefs/littlefs_FS_Driver.cpp (2 hunks)
- targets/ESP32/_FatFs/fatfs_FS_Driver.cpp (1 hunks)
- targets/ESP32/_IDF/esp32/app_main.c (1 hunks)
- targets/ESP32/_littlefs/littlefs_FS_Driver.cpp (1 hunks)
- targets/netcore/littlefs/littlefs_FS_Driver.cpp (1 hunks)
Additional comments not posted (10)
targets/ESP32/_IDF/esp32/app_main.c (2)
64-64: Verify the impact of changingreceiver_taskto core 1.Ensure that assigning
receiver_taskto core 1 does not introduce any race conditions or performance issues.
67-67: Verify the impact of changingmain_taskto core 0.Ensure that assigning
main_taskto core 0 does not introduce any race conditions or performance issues.src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_NativeFindFile.cpp (2)
28-29: Good practice: Zero-initialization offileInfo.Zero-initializing the
fileInfostructure prevents potential issues related to uninitialized memory, improving reliability.
36-36: Improvement: Update type definition toNativeFileInfo.Updating the type definition to
NativeFileInforeflects a more accurate representation of the data being handled, improving semantic clarity.targets/ESP32/_FatFs/fatfs_FS_Driver.cpp (2)
645-653: Improvement: Accurate handling of file attributes.The conditional check to determine if the file is a directory or an archive improves the accuracy of file attribute representation.
658-659: Improvement: Setting file size.Explicitly setting
fileInfo->Sizetoinfo.st_sizeensures that the file size is correctly stored in thefileInfostructure, enhancing the completeness of the information provided.targets/ChibiOS/_FatFs/fatfs_FS_Driver.cpp (1)
747-749: LGTM! Ensurest_sizeis correctly populated.The change correctly sets the
Sizeattribute of thefileInfostructure. Ensure that thest_sizefield in theinfoobject is always correctly populated.targets/ESP32/_littlefs/littlefs_FS_Driver.cpp (1)
702-704: LGTM! Ensurest_sizeis correctly populated.The change correctly sets the
Sizeattribute of thefileInfostructure. Ensure that thest_sizefield in theinfoobject is always correctly populated.targets/netcore/littlefs/littlefs_FS_Driver.cpp (1)
705-706: LGTM! Ensuresizeis correctly populated.The change correctly sets the
Sizeattribute of thefileInfostructure. Ensure that thesizefield in theinfoobject is always correctly populated.targets/ChibiOS/_littlefs/littlefs_FS_Driver.cpp (1)
736-737: LGTM! The file size assignment is correctly added.The addition of
fileInfo->Size = info.size;ensures that the file size is correctly populated in thefileInfostructure, addressing the issue of missing file size information.
- Add code to set the struct property in all drivers. - Fix managed type name for NativeFileInfo. Fix getting file size for NativeFileInfo - Add code to set the struct property in all drivers. - Fix managed type name for NativeFileInfo.
josesimoes
force-pushed
the
fix-file-sys
branch
from
85c24ef to
598b6b3
Compare
Description
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Chores