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

Feature Suggestion: Force unlinking of read only/immutable files. #74

Open
BurningEnlightenment opened this issue Mar 13, 2021 · 1 comment

Comments

@BurningEnlightenment
Copy link
Collaborator

I'm currently prototyping a command line utility around your directory reducer and noticed that by default it can't cope with filesystem entries which have FILE_ATTRIBUTE_READONLY set on windows. The algorithm also doesn't report this specific error, but (unsuccessfully) retries to delete the entries until it hits the timeout deadline.

Therefore I'd love to be able to nudge fs_handle::unlink() and friends towards trying a bit harder. Maybe something akin to flag::win_disable_unlink_emulation would fit the bill (though this isn't windows specific -- linux, BSD, and its derivatives have an immutable attribute).

However, thanks to the flexible visitor architecture it's easy enough to work around in my case and I've a hunch that the immutable file attribute is way more commonly found on windows than on any other OS. I'm attaching the workaround snippet in case someone else stumbles on this.

::BY_HANDLE_FILE_INFORMATION fileInfo{};
if (!::GetFileInformationByHandle(unmanagedHandle.h, &fileInfo))
{
    return llfio::error_info(win32_error());
}
if (!(fileInfo.dwFileAttributes & FILE_ATTRIBUTE_READONLY))
{
    return false;
}

::FILE_BASIC_INFO changedFileInfo{
        .CreationTime = as_large_integer(fileInfo.ftCreationTime),
        .LastAccessTime = as_large_integer(fileInfo.ftLastAccessTime),
        .LastWriteTime = as_large_integer(fileInfo.ftLastWriteTime),
        .ChangeTime = as_large_integer(fileInfo.ftLastWriteTime),
        .FileAttributes = fileInfo.dwFileAttributes & ~FILE_ATTRIBUTE_READONLY};
if (!::SetFileInformationByHandle(unmanagedHandle.h, FileBasicInfo,
                                    &changedFileInfo,
                                    sizeof(changedFileInfo)))
{
    return llfio::error_info(win32_error());
}
@ned14
Copy link
Owner

ned14 commented Mar 15, 2021

I'm in a tricky position on this one. On the one hand, LLFIO was never designed as a complete solution for either POSIX nor Windows, it always deliberately has exclusively targeted the common overlap between the two on the basis that as a proposed library for WG21 standardisation, that intersection is the only reasonable portion worth directly supporting.

However that isn't useful to actual end users facing platform specific differences, hence as you have observed, we deliberately expose hooks everywhere for end users to insert platform specific additional support such as for the rather non-portable Windows file attributes. (BTW I am unaware of POSIX having a similar immutable attribute, can you point me at some docs on that?)

(There is of course a huge missing chunk of support for ACLs and privileges, much of which could be portably supported across Windows and POSIX. I personally don't intend to ever implement that, as it's an awful lot of work for what isn't ever used by most end users, but I do want to leave completely open that design space in case somebody else ever comes along and wants to fill that space)

So that's the above, general, situation. Now to specifics. It is currently the case that if you unlink a file with the read only attribute set and you are on an older Windows, the read only attribute is removed and the file is deleted. Only if you are on Windows 1709 or later, and you are on NTFS, does the read only attribute prevent deletion, which of course may well be why somebody set the read only attribute.

Additionally, on Windows the directory hierarchy reduction algorithm has a custom optimised fast unlink implementation which is entirely independent of unlink. It could be taught, fairly cost free, to notice when the read only attribute is set and to unset it during reduction. However, again, if somebody sets read only, that usually means "delete everything else but not this" so I don't think that ignoring that by default is a wise move.

So, I'm not sure what to do here. My personal inclination is that end users writing code facing unknown filesystem will always have to write platform specific logic in any case, so if they want to observe/ignore file attributes, they can add logic to do so. The ignoring of read only in unlink when not on NTFS, or on older Windows, is regrettable, but in my opinion is a least worst balance of opposing tradeoffs.

Given the added exposition above, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants