<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Sep 4, 2014 at 4:49 AM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">On Wed, Sep 3, 2014 at 9:36 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>

> Hi dblaikie,<br>
><br>
> 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"`).<br>

><br>
> 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:<br>
><br>
>   std::error_code can_write(const Twine &Path, bool &result);<br>
><br>
> ... which seems redundant because `result` is set to `true` iff the error code is `success`, and ...<br>
><br>
>   std::error_code can_write(const Twine &Path);<br>
><br>
> ... which is terrible because `bool(can_write(Path))` is `true` iff the file is *not* writable, and ...<br>
><br>
>   std::error_code cannot_write(const Twine &Path);<br>
><br>
> ... which is inconsistent with other functions near here and ugly.<br>
<br>
</div>I don't think this API (in general) is a good one to have in the first<br>
place as it suffers from TOCTOU issues (as do our uses of other APIs<br>
like fs::exists). A better approach is for places like<br>
CompilerInstance.cpp is to simply open the file for writing and report<br>
the various ways that can fail instead of asking questions for which<br>
1) the answer can change subsequent to asking the question anyway, and<br>
2) the answer isn't canonical (there's a lot more reasons for not<br>
being able to write to a file than its "read-only" attribute being set<br>
on Windows, for instance).<br></blockquote><div><br></div><div>+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.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span></blockquote></div><br></div></div>