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: wrong os.File.Write return values at device end #6855

Closed
gopherbot opened this issue Dec 1, 2013 · 11 comments
Closed

os: wrong os.File.Write return values at device end #6855

gopherbot opened this issue Dec 1, 2013 · 11 comments
Milestone

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 1, 2013

by thomas.e.zander:

What steps will reproduce the problem?

On FreeBSD (see below)
1. Have a small block device ready, e.g. using mdconfig -a -t swap -s 12m
2. Do the following in a go program:

f, f_err := os.OpenFile("/dev/mdX", os.O_WRONLY|os.O_CREATE,0600) // mdX is
the device from step 1. Assuming f_err reports nil, f is valid, then do:
v := make ([]byte, 10*1024*1024)
n, err := f.Write(v) // Referring to as (i) in subsequent steps
n, err = f.Write(v) // Referring to as (ii) in subsequent steps
n, err = f.Write(v) // Referring to as (iii) in subsequent steps


What is the expected output?

According to the std lib doc for func (f *File) Write(b []byte) (n int, err error),
"Write returns a non-nil error when n != len(b)."


What do you see instead?

After line (i), n is 10MiB, err is nil. Correct.
After line (ii) n is smaller 10 MiB, err is still nil. INCORRECT: err should be non-nil
After line (iii), n is zero, err is still nil. INCORRECT: err should be non-nil


Which compiler are you using (5g, 6g, 8g, gccgo)?

6g


Which operating system are you using?

FreeBSD 9.2 STABLE amd64


Which version are you using?  (run 'go version')

go version go1.1.2 freebsd/amd64


Please provide any additional information below.

The behaviour of provided code is in accordance with the documentation if we write into
a regular file on a file system and that file system gets full.
It only happens on a short write at the end of a device.
@minux

This comment has been minimized.

Copy link
Member

@minux minux commented Dec 1, 2013

Comment 1:

what does ktrace say about this program?
what's the return value of the respective write syscall?
basically (*os.File).Write just return whatever the write syscall returns.

Status changed to WaitingForReply.

@gopherbot

This comment has been minimized.

Copy link
Author

@gopherbot gopherbot commented Dec 1, 2013

Comment 2 by thomas.e.zander:

ktrace shows the difference between write to a file and direct write to a character
device.
Writing 10 MiB blocks to a file shows the following ktrace dump:
... (omitting the successful previous writes) ...
RET   write 10485760/0xa00000
CALL  write(0x3,0xc201ec9000,0xa00000)
RET   write -1 errno 28 No space left on device
Writing to the character device shows:
... (omitting the successful previous writes) ...
CALL  write(0x3,0xc2014b8000,0xa00000)
RET   write 8388608/0x800000
CALL  write(0x3,0xc201cb8000,0x200000)
RET   write 0
The difference is:
- For a regular file on a file system, the syscall returns -1 if the write cannot
complete and sets errno 28
- For a short write at the end of a character device, the syscall returns the number of
bytes successfully written. In subsequent attempts to write to that device, zero is
returned (as can be seen above). Never -1 gets returned and never any errno is set.
FreeBSD base system tools, dd for instance, check for a short write on a device due to
this behaviour of write(2).
If os.File.Write() is supposed to behave like the documentation suggests, the method
must be extended such that it actually does set err a non-nil error when n != len(b).
@dsymonds

This comment has been minimized.

Copy link
Member

@dsymonds dsymonds commented Dec 2, 2013

Comment 3:

Can you see whether it is still behaving that way in Go 1.2?

Labels changed: added os-freebsd.

@minux

This comment has been minimized.

Copy link
Member

@minux minux commented Dec 2, 2013

Comment 4:

yes. Go 1.2 doesn't change anything in this aspect.
I don't have FreeBSD installation available, but I tested vnconfig(1) created character
device on NetBSD and confirmed that the behavior is the same (no errors reported
for (*os.File).Write). So I suspect this behavior is common to all BSDs.
The remaining problem is wether the error is in code or docs?

Labels changed: added priority-later, removed priority-triage, os-freebsd.

Status changed to Accepted.

@minux

This comment has been minimized.

Copy link
Member

@minux minux commented Dec 2, 2013

Comment 5:

And if the error is in code, we need a way to test it, preferably without using root.
@gopherbot

This comment has been minimized.

Copy link
Author

@gopherbot gopherbot commented Dec 3, 2013

Comment 6 by thomas.e.zander:

Well, if you want the code to behave identical across all platforms, then it is a code
error.
By the way, how can the code be tested right now without using root?
@minux

This comment has been minimized.

Copy link
Member

@minux minux commented Dec 3, 2013

Comment 7:

right now there isn't test for this aspect of Write.
if it's an error in code, then what error should Write return for short writes?
I'm slightly worried whether there are code that relies on this "bug".
@gopherbot

This comment has been minimized.

Copy link
Author

@gopherbot gopherbot commented Dec 3, 2013

Comment 8 by thomas.e.zander:

I would expect that this case is not really mainstream, otherwise we would have had the
issue earlier.
Also, the go documentation is crystal clear on the behaviour of Write: "Write returns a
non-nil error when n != len(b)."
So, existing go code that relies on this specific behaviour of *BSD's write syscall
should not have been written that way in the first place.
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 9:

Labels changed: added release-none, removed go1.3maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 10:

Labels changed: added repo-main.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented May 17, 2017

I believe this was fixed by https://golang.org/cl/36800043 (which was committed as part of https://golang.org/cl/36930044).

@golang golang locked and limited conversation to collaborators May 17, 2018
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
5 participants
You can’t perform that action at this time.