[PATCH] D28470: Support outputting to /dev/null

[Rui Ueyama via llvm-commits]

On Mon, Jan 9, 2017 at 12:14 PM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:
>
> > ruiu added inline comments.
> >
> >
> > ================
> > Comment at: lib/Support/FileOutputBuffer.cpp:63
> >          return EC;
> >        else
> > +        IsRegular = false;
> > ----------------
> > Remove `else`.
> Will do.
>
> > ================
> > Comment at: lib/Support/FileOutputBuffer.cpp:74
> >
> >    // Create new file in same directory but with random name.
> >    SmallString<128> TempFilePath;
> > ----------------
> > Update the comment.
> Will do.
>
> > ================
> > Comment at: lib/Support/FileOutputBuffer.cpp:92
> >
> >    sys::RemoveFileOnSignal(TempFilePath);
> >
> > ----------------
> > You want to call this only when IsRegular.
>
> Why? It is a good think to remove the temporary file in both cases.
>

Sorry, I misunderstood the code. This part looks fine.

But why do you call DontRemoveFileOnSignal on commit()? I think you want to
remove a temporary file on commit.

Does sys::fs::rename() work on two paths that reside in different
filesystems? The comment for the function says that it uses POSIX rename(),
which I believe doesn't work across filesystems.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170109/bc6f6e96/attachment.html>