<p dir="ltr">This looks good to me with a few nits:</p>
<p dir="ltr">Expand the comment to describe the windows limitation. I think most people looking at this code will have POSIX semantics in mind.</p>
<p dir="ltr">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?</p>
<p dir="ltr">Thanks a lot for fixing this.</p>
<p dir="ltr">Cheers,<br>
Rafael</p>
<div class="gmail_quote">On May 4, 2016 12:17 PM, "Cameron" <<a href="mailto:cameron@moodycamel.com">cameron@moodycamel.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">cameron314 updated the summary for this revision.<br>
cameron314 added a reviewer: rafael.<br>
cameron314 removed rL LLVM as the repository for this revision.<br>
cameron314 updated this revision to Diff 56161.<br>
cameron314 added a comment.<br>
<br>
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.)<br>
<br>
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.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D19838" rel="noreferrer" target="_blank">http://reviews.llvm.org/D19838</a><br>
<br>
Files:<br>
  include/llvm/Object/ArchiveWriter.h<br>
  lib/Object/ArchiveWriter.cpp<br>
  tools/llvm-ar/llvm-ar.cpp<br>
<br>
</blockquote></div>