[PATCH] D19838: [LLVM-AR] Fixed bug where temporary file would be left behind every time an archive was updated

[Rafael Espíndola via llvm-commits]

This looks good to me with a few nits:

Expand the comment to describe the windows limitation. I think most people
looking at this code will have POSIX semantics in mind.

Second, is it possible to add a test the currently fails on windows? What
does the renamed file looks like? Is it something we can guess with a glob?

Thanks a lot for fixing this.

Cheers,
Rafael
On May 4, 2016 12:17 PM, "Cameron" <cameron at moodycamel.com> wrote:

cameron314 updated the summary for this revision.
cameron314 added a reviewer: rafael.
cameron314 removed rL LLVM as the repository for this revision.
cameron314 updated this revision to Diff 56161.
cameron314 added a comment.

All right, I've changed the patch according to your suggestion. The buffer
is no longer linked to the `Archive` object, and is passed separately.
(Note this leaves the Archive object in an unusable state after the buffer
is removed from under it, but that llvm-ar doesn't use it after that point.)

Long term, it might be a good idea to introduce another flag to
`MemoryBuffer::getFile` to always force it to copy the file into memory
instead of mapping it. This would be less efficient most of the time, but
would fix this sort of bug which is otherwise very annoying to handle if
there's any abstraction at all in the code that consumes the buffer before
overwriting the file. It seems there's a lot of read-modify-update file
access idioms across the various tools that are prone to this problem.


http://reviews.llvm.org/D19838

Files:
  include/llvm/Object/ArchiveWriter.h
  lib/Object/ArchiveWriter.cpp
  tools/llvm-ar/llvm-ar.cpp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160504/f4a035e2/attachment.html>