[PATCH] D28010: FileOutputBuffer: support non-mmap'able file.

[Rui Ueyama via llvm-commits]

On Thu, Dec 22, 2016 at 5:52 AM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:

> Rui Ueyama <ruiu at google.com> writes:
>
> > On Wed, Dec 21, 2016 at 9:06 AM, Rafael Avila de Espindola <
> > rafael.espindola at gmail.com> wrote:
> >
> >> > +// Represents a non-memory-mapped file. We need this class
> >> > +// because not all files are mmap'able. For example, /dev/null
> >> > +// is not mmap'able.
> >>
> >> The comment is a bit confusing. The issue is that with the old code if
> >> you tried to "-o /dev/null", we would not try to mmap /dev/null. We
> >> would try to mmap a temp file and move it to /dev/null.
> >>
> >> Maybe the best way to do this is
> >> * Check the file type as you did.
> >> * Remember the result in a member variable.
> >> * If it is regular, use fs::createUniqueFile.
> >> * If it is not, use fs::createTemporaryFile, which can also be mmaped.
> >> * In commit, do a move if the target is regular or write the contents if
> >>   it is not.
> >>
> >
> > If you want do the same thing until the final rename(2) or file copy, you
> > need to create a temporary file in the same directory as the final
> > destination file (because the temporary file needs to be in the same file
> > system as the final output.) But that's not always doable. For example,
> if
> > the final output is /dev/null, /dev/ is not writable. So I guess this
> patch
> > is reasonable, no?
>
> That is why I suggested using fs::createTemporaryFile for non regular
> files. It will create a file in /tmp.
>

I don't think using fs::createTemporaryFile is an improvement. Currently,
I'm using malloc'ed memory region as a buffer, so it is probably faster
than file-system-backed file and will leave nothing if a program crashes
unexpectedly. I don't see a benefit of using createTemporaryFile.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170108/4264db54/attachment.html>