[PATCH] Add ability for can_write to indicate why the file is not writable
dblaikie at gmail.com
Thu Sep 4 10:47:09 PDT 2014
On Thu, Sep 4, 2014 at 4:49 AM, Aaron Ballman <aaron at aaronballman.com>
> On Wed, Sep 3, 2014 at 9:36 PM, Richard Smith <richard at metafoo.co.uk>
> > 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
> > 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).
+1 to all of that. Chandler & others have tales of TOCTOU stuff that scares
me sufficiently to assume that's all applicable & worth worrying about, but
perhaps Richard has a notion as to why it might not apply here? Otherwise
I'm all for making this a "try to do the thing you want (open for write,
etc) and see if it fails" rather than querying that property separately.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits