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: ambiguous mode/perm documentation for Create and OpenFile #30400

Closed
TheCount opened this issue Feb 26, 2019 · 6 comments
Closed

os: ambiguous mode/perm documentation for Create and OpenFile #30400

TheCount opened this issue Feb 26, 2019 · 6 comments

Comments

@TheCount
Copy link

@TheCount TheCount commented Feb 26, 2019

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

$ go version
go version go1.12 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/klauer/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/klauer/src/go"
GOPROXY=""
GORACE=""
GOROOT="/home/klauer/opt/go"
GOTMPDIR=""
GOTOOLDIR="/home/klauer/opt/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build968558791=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run this test program:

package main

import "os"

func main() {
        file, err := os.OpenFile("foo.dat", os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600)
        if err != nil { panic("1") }
        file.Close()

        file, err = os.Create("foo.dat")
        if err != nil { panic("2") }
        file.Close()

        file, err = os.Create("bar.dat")
        if err != nil { panic("3") }
        file.Close()

        file, err = os.OpenFile("bar.dat", os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
        if err != nil { panic("4") }
        file.Close()
}

What did you expect to see?

The documentation for os.Create says:

Create creates the named file with mode 0666 (before umask), truncating it if it already exists. […]

This reads as if mode 0666 always applies.
Likewise, the documentation for os.OpenFile says:

[…] It opens the named file with specified […] perm (before umask), if applicable. […]

This might also lead one to expect that perm is applicable with, e. g., O_CREATE and O_TRUNC.

Hence, one would expect:

$ umask
0022
$ ls -l
-rw------- 1 klauer klauer       0 Feb 26 10:53 bar.dat
-rw-r--r-- 1 klauer klauer       0 Feb 26 10:53 foo.dat

What did you see instead?

The actual behaviour is in line with the analogous flags for the open() function from the POSIX standard:

$ umask
0022
$ ls -l
-rw-r--r-- 1 klauer klauer       0 Feb 26 10:53 bar.dat
-rw------- 1 klauer klauer       0 Feb 26 10:53 foo.dat

The POSIX standard makes it explicitly clear that the mode remains unchanged when the file already exists:

O_CREAT If the file exists, this flag has no effect except as noted under O_EXCL below.

O_TRUNC If the file exists and is a regular file, and the file is successfully opened O_RDWR or O_WRONLY, its length shall be truncated to 0, and the mode and owner shall be unchanged.

I suggest the go documentation is likewise explicit. For example I would change the os.Create docs:

Create creates the named file with mode 0666 (before umask), truncating it if it already exists.

to:

Create creates the named file with mode 0666 (before umask) if it does not already exist. If it already exists, it is truncated and the mode remains unchanged.

and the os.OpenFile docs:

It opens the named file with specified flag (O_RDONLY etc.) and perm (before umask), if applicable.

to

It opens the named file with specified flag (O_RDONLY etc.). The perm is applied (before umask) only if a new file is created, and is otherwise ignored.

@TheCount
Copy link
Author

@TheCount TheCount commented Feb 26, 2019

It just occurred to me that os.MkdirAll might suffer from the same ambiguity, but I just checked and this is not the case.

@agnivade agnivade changed the title os.Create, os.OpenFile: ambiguous mode/perm documentation os: ambiguous mode/perm documentation for Create and OpenFile Feb 26, 2019
@agnivade
Copy link
Contributor

@agnivade agnivade commented Feb 26, 2019

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 26, 2019

Change https://golang.org/cl/163744 mentions this issue: os: clarify that mode argument is only used if file is created

@TheCount
Copy link
Author

@TheCount TheCount commented Feb 26, 2019

Cool, thanks! Commit 3995bc007d94f6923d79f3b18089dab2caaa4580 on googlesource.com contains two typos:

  • son → on
  • umark → umask

I can't seem to comment directly on that commit.

@agnivade
Copy link
Contributor

@agnivade agnivade commented Feb 26, 2019

One cannot comment directly on the googlesource site. Please feel free to comment on gerrit, which is our code review tool. Use this link - https://go-review.googlesource.com/c/go/+/163744/

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Feb 26, 2019

Thanks, updated the CL with typo fixes.

@gopherbot gopherbot closed this in 4b05dc9 Feb 26, 2019
@golang golang locked and limited conversation to collaborators Feb 26, 2020
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
4 participants
You can’t perform that action at this time.