Page MenuHomePhabricator

Improve reliability of file renaming in Windows

Authored by dyung on Mar 4 2016, 5:50 PM.



Improve the reliability of file renaming in Windows by having the compiler retry the rename operation on 3 error conditions of ReplaceFileW() that it was previously bailing out on.

Diff Detail


Event Timeline

dyung updated this revision to Diff 49863.Mar 4 2016, 5:50 PM
dyung retitled this revision from to Improve reliability of file renaming in Windows.
dyung updated this object.
dyung added a reviewer: llvm-commits.
dyung edited reviewers, added: Bigcheese, gbedwell; removed: llvm-commits.Mar 4 2016, 5:55 PM
dyung added a subscriber: llvm-commits.
asl added a subscriber: asl.Mar 7 2016, 12:41 PM
vsk added a subscriber: vsk.Mar 16 2016, 10:24 PM
vsk added inline comments.
275 ↗(On Diff #49863)

Does ReplaceError need to be declared a DWORD somewhere? I'm not familiar with this code, I could just be missing something.

dyung updated this revision to Diff 51006.Mar 17 2016, 6:39 PM

Fixed missing declaration identified by vsk.

rnk added a reviewer: rnk.Mar 18 2016, 8:53 AM
rnk added inline comments.Mar 18 2016, 9:02 AM
271 ↗(On Diff #51006)

These begin() calls should be data() calls.

281 ↗(On Diff #51006)

"The file to be replaced still exists with a different name."

Should we do anything about that?

dyung updated this revision to Diff 51124.Mar 20 2016, 1:00 AM

Changed .begin() calls to .data() calls as suggested in review.

dyung added inline comments.Mar 20 2016, 1:12 AM
281 ↗(On Diff #51124)

We felt that this situation was not possible in this case since we call ReplaceFileW with NULL as the value for lpBackupFileName, so we felt that no backup file would be made. Because of that, if this error occurs, we thought the result would be the same as the ERROR_UNABLE_TO_MOVE_REPLACEMENT, so we would handle it the same way.

rnk accepted this revision.Mar 21 2016, 9:21 AM
rnk edited edge metadata.


281 ↗(On Diff #51124)

I can agree with that interpretation. :)

This revision is now accepted and ready to land.Mar 21 2016, 9:21 AM
This revision was automatically updated to reflect the committed changes.