[PATCH] D42925: Call FlushFileBuffers on readwrite file mappings.

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

In https://reviews.llvm.org/D42925#998354, @rnk wrote:

> One concern I have is that this is a common pattern on Linux:
>
>   int fd = open(...);
>   void *mem = mmap(fd, ...);
>   close(fd);
>   // use mem
>   
>
> Are we reasonably confident that LLVM doesn't have this pattern anywhere in it? i.e. we never store the FD and then try to use it after closing it?


We don't do that anywhere *at the moment* that I can find, but I suppose it's a concern for future proofing this code.  Someone could come along and add code like this after the fact.  Currently the only place where we open a `mapped_file_region` for readwrite is in `OnDiskBuffer`, which stores a `TempFile` and then renames it atomically to the final path.  We could put the flush logic inside of `TempFile::keep()` so that it flushes it before renaming.  That might be a better option, although it wouldn't catch the case where someone comes along later and tries to use a `mapped_file_region` for something else.  Perhaps it's ok for the time being though?


https://reviews.llvm.org/D42925