<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 11, 2014 at 2:55 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 11 September 2014 17:18, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>> wrote:<br>
> -      bool Exists;<br>
> -      if (!sys::fs::exists(LockFileName.str(), Exists) && !Exists) {<br>
> +      if (sys::fs::access(LockFileName.c_str(), sys::fs::AccessMode::Exist)<br>
> ==<br>
> +          errc::no_such_file_or_directory) {<br>
><br>
> Is there a reason you didn't just use the `bool exists(const Twine &Path)`<br>
> helper here?<br>
<br>
</span>I didn't want to include any semantic change in the patch. The<br>
original code would take the branch only if sys::fs::exists returned<br>
no error. Using the simpler version would take the branch if there was<br>
an error.<br>
<br>
To see the difference consider something like "permission denied". It<br>
would not take the branch originally and will not with the way it is<br>
currently written, but would with<br>
<br>
if (!sys::fs::exists(LockFileName.c_str()))<br>
<br>
Now, it may be a bug in the code, but it is probably better to fix<br>
that in another patch.<br></blockquote><div><br></div><div>That's incredibly subtle. Can we rename exists to better reflect that?</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div></div>