Fix unprovoked symlink errors in directory_entry - #353
Conversation
|
I'd like to let you know that there is a massive change to library headers coming soon, that will likely make PRs against headers non-applicable. You may want to hold off the work on this. |
|
@Lastique Thank you for the heads up. I've converted to draft for now. |
5a366ca to
15dbc1d
Compare
Since boostorg@d508d49, a number of functions in directory_entry's public interface call directory_entry::refresh_impl, which updates the cached file status comprehensively. On POSIX systems, for example, both lstat and stat are called for symlinks, and stat errors are reported to the caller. As a result, e.g. checking the status of a symlink with symlink_status produces an error if the symlink target is gone or if access to it is denied. In v4, this also affects the constructor overload that takes an error_code, i.e. constructing a directory_entry with a path to a broken symlink produces an error. Add a parameter to refresh_impl allowing to configure what the caller is interested in, thus avoiding false positives.
15dbc1d to
76bf91a
Compare
|
I've rebased around 2466fc1 (I assumed this was the change to library headers). |
| BOOST_FILESYSTEM_DECL void refresh_impl( | ||
| system::error_code* ec = nullptr, | ||
| refresh_mode mode = refresh_mode::follow_lenient) const; | ||
| void refresh_impl(refresh_mode mode) const { refresh_impl(nullptr, mode); } |
There was a problem hiding this comment.
I'd rather there was a single refresh_impl, with the first argument of refresh_mode/update_mask (with no default) and the second one of error_core* (with nullptr default). This follows the existing convention.
| { | ||
| no_follow, | ||
| follow, | ||
| follow_lenient |
There was a problem hiding this comment.
I'm not sure lenient is needed. In C++ standard, std::filesystem::directory_entry constructor and assign are specified to call refresh, i.e. those APIs are expected to fail if status fails. Other APIs that rely on status, obviously, should also fail if status fails. The only APIs that only require symlink status are symlink_status and is_symlink, and those APIs may not update the cached status.
I think, a better way to do this is change refresh_mode to update_mask, which would be a bit mask of which statuses to update (update_status, update_symlink_status). symlink_status and is_symlink would only specify update_symlink_status and every other API would specify both.
Fix unprovoked symlink errors in directory_entry Since boostorg@d508d49, a number of functions in directory_entry's public interface call directory_entry::refresh_impl, which updates the cached file status. This happens comprehensively; on POSIX systems, for example, both lstat and stat are called for symlinks, and stat errors are reported to the caller. As a result, e.g. checking the status of a symlink with symlink_status produces an error if the symlink target is gone or if access to it is denied. Add a parameter to refresh_impl allowing to configure what the caller is interested in, thus avoiding false positives. Note that only symlink_status, symlink_file_type and the APIs implemented in terms of them skip the m_status update. The other APIs still update both statuses, and report errors accordingly.
Remove redundant comment
Get rid of the refresh_impl overload, makes sure error_code is the last parameter.
Introduce enumerator to use as a shorthand
|
@Lastique Thanks. I pushed some fixups, in separate commits to make them hopefully easier to review. |
|
Thanks. |
2 participants
Closes #352.
Also adds tests exercising
directory_entry.