Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: Make os.Rename act more similar to *NIX on Windows #10773

Closed
cyisfor opened this issue May 11, 2015 · 2 comments
Closed

os: Make os.Rename act more similar to *NIX on Windows #10773

cyisfor opened this issue May 11, 2015 · 2 comments

Comments

@cyisfor
Copy link

@cyisfor cyisfor commented May 11, 2015

I was toying with atomic file operations, and noticed os.Rename for the Microsoft implementation uses MoveFileW, yet MoveFileExW exists which accepts a flags argument. os.Rename on everything not Windows (and plan9) is "atomic," meaning that it replaces the destination if it exists, with no opportunity to crash in user space. In Microsoft, if the destination exists, the os.Rename operation fails and the programmer is expected to be able to remove and replace the destination themselves without any way to prevent race conditions or interruptions halfway through.

This will confuse any programmers on Microsoft, performing needless existence checks when calling os.Rename on non-Microsoft, and any programmers not on Microsoft will be rather miffed at being unable to do safe file renames on Microsoft.

The non-Microsoft behavior can be achieved in Microsoft using MoveFileExW (I think) by passing certain flags to the procedure. So I recommend that the Go language adopt MoveFileExW as the system call for Microsoft, when using os.Rename, so that os.Rename behaves the same on Microsoft, and off Microsoft.

It should be about a 2 line patch, but I have no way to test whether it works correctly or not. Just throwing it out there.

using go 1.5 btw, from a recent git pull
go version devel +e8fc93e Fri May 8 04:23:43 2015 +0000 linux/amd64

@cyisfor
Copy link
Author

@cyisfor cyisfor commented May 11, 2015

diff --git a/src/syscall/syscall_windows.go b/src/syscall/syscall_windows.go
index feb329f..55f2a50 100644
--- a/src/syscall/syscall_windows.go
+++ b/src/syscall/syscall_windows.go
@@ -148,7 +148,7 @@ func NewCallbackCDecl(fn interface{}) uintptr {
 //sys  CreateDirectory(path *uint16, sa *SecurityAttributes) (err error) = CreateDirectoryW
 //sys  RemoveDirectory(path *uint16) (err error) = RemoveDirectoryW
 //sys  DeleteFile(path *uint16) (err error) = DeleteFileW
-//sys  MoveFile(from *uint16, to *uint16) (err error) = MoveFileW
+//sys  MoveFileEx(from *uint16, to *uint16, flags uint32) (err error) = MoveFileExW
 //sys  GetComputerName(buf *uint16, n *uint32) (err error) = GetComputerNameW
 //sys  SetEndOfFile(handle Handle) (err error)
 //sys  GetSystemTimeAsFileTime(time *Filetime)
@@ -409,7 +409,11 @@ func Rename(oldpath, newpath string) (err error) {
    if err != nil {
        return err
    }
-   return MoveFile(from, to)
+   // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365240%28v=vs.85%29.aspx
+   // 1 = MOVEFILE_REPLACE_EXISTING
+   // 2 = MOVEFILE_COPY_ALLOWED (not *NIX behavior)
+   // 8 = MOVEFILE_WRITE_THROUGH
+   return MoveFileEx(from, to, 1|8)
 }

 func ComputerName() (name string, err error) {
@minux
Copy link
Member

@minux minux commented May 11, 2015

Dup of #8914.

@minux minux closed this May 11, 2015
@mikioh mikioh changed the title Make os.Rename act more similar to *NIX on Windows os: Make os.Rename act more similar to *NIX on Windows May 11, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.