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 filesystem's handling of nonexistent network paths emitting ERROR_INVALID_NAME #800

Conversation

StephanTLavavej
Copy link
Member

On my machine, P0218R1_filesystem was failing whenever it tested nonexistent network paths. (I'm not sure why nobody else has observed this yet; I'm just running a normal Microsoft corpnet installation of Windows 10 Enterprise.)

I debugged into this, and observed that when __std_fs_get_stats() calls GetFileAttributesExW(), it fails with ERROR_INVALID_NAME:

STL/stl/src/filesystem.cpp

Lines 919 to 921 in 65d98ff

if (!GetFileAttributesExW(_Path, GetFileExInfoStandard, &_Data)) {
return __std_win_error{GetLastError()};
}

This is returned unchanged by _File_size():

STL/stl/inc/filesystem

Lines 3377 to 3388 in 65d98ff

_NODISCARD inline __std_win_error _File_size(const path& _Path, uintmax_t& _Result) noexcept {
__std_fs_stats _Stats;
const auto _Error = __std_fs_get_stats(
_Path.c_str(), &_Stats, __std_fs_stats_flags::_Follow_symlinks | __std_fs_stats_flags::_File_size);
if (_Error == __std_win_error::_Success) {
_Result = _Stats._File_size;
} else {
_Result = static_cast<uintmax_t>(-1);
}
return _Error;
}

The test expects file_size() to return bad_file_size, which it does, but it also expects ec == errc::no_such_file_or_directory, which fails:

for (auto&& nonexistent : nonexistentPaths) {
EXPECT(bad_file_size == file_size(nonexistent, ec));
EXPECT(ec == errc::no_such_file_or_directory);
}

This is because we map ERROR_INVALID_NAME to errc::invalid_argument:

{ERROR_INVALID_NAME, errc::invalid_argument},

The Windows documentation for System Error Codes describes ERROR_INVALID_NAME as "The filename, directory name, or volume label syntax is incorrect." so this is specific to filesystem operations; it isn't a general error code for invalid arguments.

Additionally, __std_is_file_not_found() already considers __std_win_error::_Invalid_name (our synonym for ERROR_INVALID_NAME) to be a "file not found" error code:

_NODISCARD inline bool __std_is_file_not_found(const __std_win_error _Error) noexcept {
switch (_Error) {
case __std_win_error::_File_not_found:
case __std_win_error::_Path_not_found:
case __std_win_error::_Error_bad_netpath:
case __std_win_error::_Invalid_name:
return true;
default:
return false;
}
}

The other 3 error codes are already mapped to errc::no_such_file_or_directory:

{ERROR_FILE_NOT_FOUND, errc::no_such_file_or_directory},

{ERROR_PATH_NOT_FOUND, errc::no_such_file_or_directory},

{ERROR_BAD_NETPATH, errc::no_such_file_or_directory},

Changing the mapping of ERROR_INVALID_NAME (and updating the test's direct inspection of ec.value()) fixes this bug.

This is a behavioral change (similar to #406 and #616, although they added new mappings). As this fixes the behavior of <filesystem>, I believe it is necessary.

@StephanTLavavej StephanTLavavej added the bug Something isn't working label May 6, 2020
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner May 6, 2020 03:25
@StephanTLavavej StephanTLavavej self-assigned this May 6, 2020
@StephanTLavavej StephanTLavavej merged commit 5e3f23d into microsoft:master May 7, 2020
@StephanTLavavej StephanTLavavej deleted the filesystem_error_invalid_name branch May 7, 2020 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants