-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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.
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
AP_HAL_Linux: file descriptor leaking and other issues in "Storage" of Linux #28052
Conversation
junan76
commented
Sep 9, 2024
- Fixed the fd leaking issue in "_storage_create" found by @peterbarker
- Remove the unnecessary call of "unlinkat" in "_storage_create"
- Simplify the implementation of "init"
…f Linux. - Fixed the fd leaking issue in "_storage_create" found by @peterbarker - Remove the unnecessary call of "unlinkat" in "_storage_create" - Simplify the implementation of "init" Signed-off-by: junan <[email protected]>
This looks like a nice change. How has it been tested? |
I tested it on beaglebone black board using gdb, will post the screenshots this evening, you can check the result then. |
@@ -145,26 +145,16 @@ void Storage::init() | |||
dpath = HAL_BOARD_STORAGE_DIRECTORY; | |||
} | |||
|
|||
int fd = open(dpath, O_RDWR|O_CLOEXEC); | |||
int fd = _storage_create(dpath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should rename this to_storage_open_or_create
. Just wondering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can rename it to _storage_open
maybe, we call openat
with O_CREAT
flag which will create the file if it does not exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hi peter, What should I do with the |
Just tried it on a Navigator and and everything worked as expectd |