[PATCH] Add ability for can_write to indicate why the file is not writable

Aaron Ballman aaron at aaronballman.com
Thu Sep 4 04:49:09 PDT 2014


On Wed, Sep 3, 2014 at 9:36 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> Hi dblaikie,
>
> Add an optional output parameter to `can_write` to allow it to indicate why a file is not considered writable (along with a sample use from Clang where we currently produce stupid diagnostics saying `"can't open output file: Success"`).
>
> I'm not entirely sure this is the right design, and in particular it's a somewhat different interface from the other functions in this file. I also considered:
>
>   std::error_code can_write(const Twine &Path, bool &result);
>
> ... which seems redundant because `result` is set to `true` iff the error code is `success`, and ...
>
>   std::error_code can_write(const Twine &Path);
>
> ... which is terrible because `bool(can_write(Path))` is `true` iff the file is *not* writable, and ...
>
>   std::error_code cannot_write(const Twine &Path);
>
> ... which is inconsistent with other functions near here and ugly.

I don't think this API (in general) is a good one to have in the first
place as it suffers from TOCTOU issues (as do our uses of other APIs
like fs::exists). A better approach is for places like
CompilerInstance.cpp is to simply open the file for writing and report
the various ways that can fail instead of asking questions for which
1) the answer can change subsequent to asking the question anyway, and
2) the answer isn't canonical (there's a lot more reasons for not
being able to write to a file than its "read-only" attribute being set
on Windows, for instance).

~Aaron



More information about the llvm-commits mailing list