[llvm] r217625 - Misc cleanups to the FileSytem api.

[Sean Silva]

On Thu, Sep 11, 2014 at 2:55 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> On 11 September 2014 17:18, Sean Silva <chisophugis at gmail.com> wrote:
> > -      bool Exists;
> > -      if (!sys::fs::exists(LockFileName.str(), Exists) && !Exists) {
> > +      if (sys::fs::access(LockFileName.c_str(),
> sys::fs::AccessMode::Exist)
> > ==
> > +          errc::no_such_file_or_directory) {
> >
> > Is there a reason you didn't just use the `bool exists(const Twine
> &Path)`
> > helper here?
>
> I didn't want to include any semantic change in the patch. The
> original code would take the branch only if sys::fs::exists returned
> no error. Using the simpler version would take the branch if there was
> an error.
>
> To see the difference consider something like "permission denied". It
> would not take the branch originally and will not with the way it is
> currently written, but would with
>
> if (!sys::fs::exists(LockFileName.c_str()))
>
> Now, it may be a bug in the code, but it is probably better to fix
> that in another patch.
>

That's incredibly subtle. Can we rename exists to better reflect that?

-- Sean Silva


>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140911/b26a52bf/attachment.html>