os: inconsistent cross-platform behavior: os.OpenFile/os.Write* #21322

prasannavl · 2017-08-06T01:54:14Z

What version of Go are you using (`go version`)?

go version go1.8.3 windows/amd64
go version go1.8.3 linux/amd64

What operating system and processor architecture are you using (`go env`)?

Skipping as the details are covered below.

What did you do?

In Windows, the following program will happily end up writing to the file with no issues.

package main

import "os"
import "fmt"

func main() {
	flag := os.O_CREATE
	file, err := os.OpenFile("test.log", flag, os.FileMode(0644))
	if err != nil {
		fmt.Println(err)
	}
	defer file.Close()
	n, err := file.WriteString("Hello")
	if err != nil {
		fmt.Println(err)
	}
	fmt.Printf("Written: %v\r\n", n)
	fmt.Println("Good-bye.")
}

However, in linux this will fail to write with a bad file descriptor error, since there's no write permission that's given. The os.O_RDWR/WRONLY is required during OpenFile for the write to succeed.

Possible solutions

Document it.
Do run time checks to ensure consistent behavior.

The text was updated successfully, but these errors were encountered:

davecheney · 2017-08-06T02:28:09Z

Or use os.Create which deals with these cross platform issues.

…

On Sun, 6 Aug 2017, 11:54 Prasanna V. Loganathar ***@***.***> wrote: What version of Go are you using (go version)? go version go1.8.3 windows/amd64 go version go1.8.3 linux/amd64 What operating system and processor architecture are you using (go env)? Skipping as the details are covered below. What did you do? In Windows, the following program will happily end up writing to the file with no issues. package main import "os" import "fmt" func main() { flag := os.O_CREATE file, err := os.OpenFile("test.log", flag, os.FileMode(0644)) if err != nil { fmt.Println(err) } defer file.Close() n, err := file.WriteString("Hello") if err != nil { fmt.Println(err) } fmt.Printf("Written: %v\r\n", n) fmt.Println("Good-bye.") } However, in linux this will fail to write with a bad file descriptor error, since there's no write permission that's given. The os.O_RDWR/WRONLY is required during OpenFile for the write to succeed. Possible solutions 1. Document it. 2. Do run time checks to ensure consistent behavior. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#21322>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAcA2qI6hP5wI5er7hO7kgis-Rl6a9Tks5sVRzIgaJpZM4OunKf> .

prasannavl · 2017-08-06T02:31:32Z

@davecheney, you can't, if you need to create the file it doesn't exist, or just append, which is quite common.. logging for instance.

prasannavl · 2017-08-06T12:47:09Z

Note: This is because, on Windows, the syscall package actually goes one step further here: https://github.com/golang/go/blob/master/src/syscall/syscall_windows.go#L269

To ensure that when APPEND flag is given, it also add the write perm. Should this behavior be added in Linux also? It make perfect sense semantically.

ianlancetaylor · 2017-08-09T22:59:41Z

To save looking through the link, the issue is that on Windows syscall.Open sets the write flag for the Windows file when the mode includes the syscall.O_CREAT flag. This does not happen for syscall.Open on other operating systems.

We could change Windows, which could potentially break existing working programs on Windows.

We could change other systems in the syscall package. This would be annoying to code but would be unlikely to break anybody. It would remove a small and probably unimportant layer of protection from programs that use os.O_CREATE|os.O_EXCL, without os.O_WRONLY to get an unwriteable lock file.

We could make the similar change in the os package, which would be less annoying but have the same minor drawbacks.

We could simply document it.

We could ignore it.

prasannavl · 2017-08-28T13:51:19Z

I think a better way to handle this would be to take a page from how Rust does this. OpenFile doesn't take any arguments other than the path, with sensible defaults, and OpenOptions - a builder pattern that handles the OS specific ways.

I think this is far superior design. I find myself have to specify a sensible default for fileMode which is a completely useless parameter in Windows.

alexbrainman · 2017-08-28T23:45:28Z

@ianlancetaylor I don't see anything sinister in any of your proposals.

I like your last suggestion (to ignore this issue) the best. Go on Windows does many things differently then on other OSes. And this is just one of them. I don't see why it is important for the program above to behave the same on different OSes. Last os.FileMode(0644) parameter is meaningless on Windows - why we should care about it?

Alex

prasannavl · 2017-09-01T09:36:20Z

I don't see why it is important for the program above to behave the same on different OSes

That's rather a saddening comment from a team member of a language which happens to have cross-platform as one of it's core values.

Last os.FileMode(0644) parameter is meaningless on Windows - why we should care about it?

Since it does nothing in windows, you can effectively ignore fileMode - and it's fairly common to develop on windows, and deploy on linux. (I have been doing this for as long as I can remember and now thanks to WSL - I barely ever use linux on desktops - and I suspect there would be a good number of us)

Now, you write os.OpenFile("somefile", os.O_APPEND | os.O_CREATE, 0) - It works perfectly on Windows. Deploy it on Linux, everything falls over silently (that's worse) - You'll get a file that will NEVER be written to. Despite being a static language, the lack of static enums and no modes in the std lib doesn't help the case.

Also, above point, again.

mattn · 2017-09-01T09:49:47Z

I prefer to simply document it. package os's purpose is not to hide every detail, just to provide a consistent interface.

https://groups.google.com/forum/#!msg/golang-dev/TlLZoZjJuCk/oQKXUkzGhoEJ

prasannavl · 2017-09-01T10:07:42Z

@mattn, I agree with the comment on the link and simply documenting it as an immediate resolution to the issue as well - I can take this up, and submit a CL in a few days when I get time.

That being said, I also wonder if chalking it as we cannot deal with every detail, is the best strategy here especially when a std lib apis like Rust's has clearly demonstrated a better (and simpler) design that covers both sides of the design. A builder pattern like that, will also end up removing a bunch of complicated flags case handling in windows and make the overall code nicer and more importantly predictable and intuitively correct - which are 2 things I find very important in std lib design.

I wonder if fileMode being a complete dummy parameter on windows, can even be considered a "consistent interface" here.

Besides, I wouldn't have a problem with this, if file handing was simply delegated to the OS on all the OS - which implies consistent and predictable behavior. But here, Go's std library doesn't do that as well. It tries to - neither here nor there - which makes things unpredictable and hard to determine.

alexbrainman · 2017-09-08T03:28:08Z

That's rather a saddening comment from a team member of a language which happens to have cross-platform as one of it's core values.

Cross-platform is important, but there are other things that Go developers care about too.

Since it does nothing in windows, you can effectively ignore fileMode - and it's fairly common to develop on windows, and deploy on linux.

It would be nice if we could just deploy code on another OS and it will work. But I doubt it is possible without real testing on real OS.

I agree with the comment on the link and simply documenting it as an immediate resolution to the issue as well

I am not sure I like any new documentation here. What are you going to say? I don't see that documentation affecting many people, but everyone will have to read and comprehend new text.

I wonder if fileMode being a complete dummy parameter on windows, can even be considered a "consistent interface" here.

Besides, I wouldn't have a problem with this, if file handing was simply delegated to the OS on all the OS - which implies consistent and predictable behavior. But here, Go's std library doesn't do that as well. It tries to - neither here nor there - which makes things unpredictable and hard to determine.

You are talking about things that cannot be changed now. Maybe for Go2? Make suggestion for Go2, if you have some great ideas.

Alex

rsc · 2017-10-30T20:34:16Z

I do think that OpenFile and these flags are a bit too low-level, which is why we introduced os.Open and os.Create. It's possible we need another so that OpenFile is essentially never needed; that is, needed only when you really need low-level system flags. But that's not where we are today.

The current docs for the constants say:

Flags to OpenFile wrapping those of the underlying system. Not all flags may be implemented on a given system.

Let's add: "Each call to OpenFile should specify exactly one of O_RDONLY, O_WRONLY, or O_RDWR."

gopherbot · 2017-10-30T23:32:58Z

Change https://golang.org/cl/74550 mentions this issue: os: clarify that OpenFile reqires one of O_RDONLY/O_WRONLY/O_RDWR

gopherbot · 2017-11-01T20:38:55Z

Change https://golang.org/cl/75250 mentions this issue: os: rearrange OpenFile Flags doc

Updates #21322 Change-Id: Ib03ee9dbe1b44c2fecd51f2f2c23a88482158e7e Reviewed-on: https://go-review.googlesource.com/75250 Reviewed-by: Rob Pike <r@golang.org>

ALTree changed the title Inconsistent cross-platform behavior: os.OpenFile/os.Write* os: inconsistent cross-platform behavior: os.OpenFile/os.Write* Aug 6, 2017

ianlancetaylor added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 9, 2017

ianlancetaylor added this to the Go1.10 milestone Aug 9, 2017

prasannavl mentioned this issue Aug 28, 2017

proposal: Go 2: function overloading #21659

Closed

rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 30, 2017

gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 30, 2017

gopherbot closed this as completed in 94d9371 Oct 31, 2017

gopherbot pushed a commit that referenced this issue Nov 1, 2017

os: rearrange OpenFile Flags doc

86b7721

Updates #21322 Change-Id: Ib03ee9dbe1b44c2fecd51f2f2c23a88482158e7e Reviewed-on: https://go-review.googlesource.com/75250 Reviewed-by: Rob Pike <r@golang.org>

golang locked and limited conversation to collaborators Nov 1, 2018

gopherbot added the FrozenDueToAge label Nov 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

os: inconsistent cross-platform behavior: os.OpenFile/os.Write* #21322

os: inconsistent cross-platform behavior: os.OpenFile/os.Write* #21322

prasannavl commented Aug 6, 2017 •

edited

davecheney commented Aug 6, 2017 via email

prasannavl commented Aug 6, 2017 •

edited

prasannavl commented Aug 6, 2017 •

edited

ianlancetaylor commented Aug 9, 2017

prasannavl commented Aug 28, 2017 •

edited

alexbrainman commented Aug 28, 2017

prasannavl commented Sep 1, 2017 •

edited

mattn commented Sep 1, 2017

prasannavl commented Sep 1, 2017 •

edited

alexbrainman commented Sep 8, 2017

rsc commented Oct 30, 2017

gopherbot commented Oct 30, 2017

gopherbot commented Nov 1, 2017

os: inconsistent cross-platform behavior: os.OpenFile/os.Write* #21322

os: inconsistent cross-platform behavior: os.OpenFile/os.Write* #21322

Comments

prasannavl commented Aug 6, 2017 • edited

What version of Go are you using (go version)?

What operating system and processor architecture are you using (go env)?

What did you do?

Possible solutions

davecheney commented Aug 6, 2017 via email

prasannavl commented Aug 6, 2017 • edited

prasannavl commented Aug 6, 2017 • edited

ianlancetaylor commented Aug 9, 2017

prasannavl commented Aug 28, 2017 • edited

alexbrainman commented Aug 28, 2017

prasannavl commented Sep 1, 2017 • edited

mattn commented Sep 1, 2017

prasannavl commented Sep 1, 2017 • edited

alexbrainman commented Sep 8, 2017

rsc commented Oct 30, 2017

gopherbot commented Oct 30, 2017

gopherbot commented Nov 1, 2017

prasannavl commented Aug 6, 2017 •

edited

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

prasannavl commented Aug 6, 2017 •

edited

prasannavl commented Aug 6, 2017 •

edited

prasannavl commented Aug 28, 2017 •

edited

prasannavl commented Sep 1, 2017 •

edited

prasannavl commented Sep 1, 2017 •

edited