Skip to content
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

Fix & Improve doc comment on some structures #167

Open
minghu6 opened this issue Aug 21, 2022 · 0 comments
Open

Fix & Improve doc comment on some structures #167

minghu6 opened this issue Aug 21, 2022 · 0 comments

Comments

@minghu6
Copy link

minghu6 commented Aug 21, 2022

Fix staled comments on DirList, DirEntry, IntoIter

It's said that these structure relies on std::fs::DirEntry which is outdated since we use our crate::dent::DirEntry

Supply additional comment for difference between std DirEntry and our DirEntry

I've no idea if we've noticed that std DirEntry hold a reference of opened dir as the documented mentioned.

In a detail, on Unix implementaions:

ReadDir holds an Arc of InnerReadDir, and it shares it with DirEntry.

readdir

https://github.com/rust-lang/rust/blob/aa8e761defc245d08d2cf226786def8a8bb56e53/library/std/src/sys/unix/fs.rs#L1296

ReadDir

https://github.com/rust-lang/rust/blob/aa8e761defc245d08d2cf226786def8a8bb56e53/library/std/src/sys/unix/fs.rs#L231

Impl Iterator for ReadDir

https://github.com/rust-lang/rust/blob/aa8e761defc245d08d2cf226786def8a8bb56e53/library/std/src/sys/unix/fs.rs#L587

DirEntry

https://github.com/rust-lang/rust/blob/aa8e761defc245d08d2cf226786def8a8bb56e53/library/std/src/sys/unix/fs.rs#L257

The InnderReadDir holds the Dir which is an wrapper of platform native opaque DIR.

DIR

https://github.com/rust-lang/rust/blob/aa8e761defc245d08d2cf226786def8a8bb56e53/library/std/src/sys/unix/fs.rs#L244

When the InnerReadDir is dropped, it calls native closedir would close underlying fd associated.

impl Drop for Dir

https://github.com/rust-lang/rust/blob/aa8e761defc245d08d2cf226786def8a8bb56e53/library/std/src/sys/unix/fs.rs#L687

closedir

https://man7.org/linux/man-pages/man3/closedir.3.html
https://pubs.opengroup.org/onlinepubs/9699919799.2016edition/toc.htm

In other words, If we keep the DirEntry, the InnerReader inner Arc wouldn't be dropped, that's: directory fd wouldn't be released even through we drop ReadDir through max_open control ! So, we could encounter Too many open files error when using collect on the iteration when walking a big enough directory !

However, luckily, we use our impl DirEntry instead of std version since it doesnt hold the fd reference. We just need to document it as additional doc comment before our DirEntry!

@minghu6 minghu6 changed the title Fix & Improve doc comment on some structure Fix & Improve doc comment on some structures Aug 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant