Hello seed@mail.nanosouffle.net, rsc@golang.org (cc: golang-dev@googlegroups.com, lucio.dere@gmail.com), I'd like you to review this change to https://code.google.com/p/go/
11 years, 5 months ago
(2012-10-12 20:52:24 UTC)
#1
11 years, 4 months ago
(2012-11-08 04:55:28 UTC)
#6
https://codereview.appspot.com/6157045/diff/9001/src/pkg/os/dir_plan9.go
File src/pkg/os/dir_plan9.go (right):
https://codereview.appspot.com/6157045/diff/9001/src/pkg/os/dir_plan9.go#newc...
src/pkg/os/dir_plan9.go:12: func (file *File) readdir(n int) (fi []FileInfo, err
error) {
On 2012/11/08 04:30:21, dfc wrote:
> drop the named return values.
Done.
https://codereview.appspot.com/6157045/diff/9001/src/pkg/os/dir_plan9.go#newc...
src/pkg/os/dir_plan9.go:23: fi = make([]FileInfo, 0, size) // Empty with room to
grow.
On 2012/11/08 04:30:21, dfc wrote:
> fi :=
Done.
https://codereview.appspot.com/6157045/diff/9001/src/pkg/os/dir_plan9.go#newc...
src/pkg/os/dir_plan9.go:29: if err == io.EOF {
On 2012/11/08 04:30:21, dfc wrote:
> be careful here, err was shadowed by the return value
Thanks.
https://codereview.appspot.com/6157045/diff/9001/src/pkg/syscall/dir_plan9.go
File src/pkg/syscall/dir_plan9.go (right):
https://codereview.appspot.com/6157045/diff/9001/src/pkg/syscall/dir_plan9.go...
src/pkg/syscall/dir_plan9.go:55: // Null assigns special "don't-touch" values to
members of d to
On 2012/11/08 04:30:21, dfc wrote:
> is "don't-touch" a known value ? if not "don't touch" might be simpler.
Yeah. I'm not happy about this comment. Any ideas for a better wording?
These values are documented in the stat(5) man page:
A wstat request can avoid modifying some properties of the file by
providing explicit "don't touch" values in the stat data that is sent:
zero-length strings for text values and the maximum unsigned value of
appropriate size for integral values. As a special case, if all the
elements of the directory entry in a Twstat message are "don't touch"
values, the server may interpret it as a request to guarantee that the
contents of the associated file are committed to stable storage before
the Rwstat message is returned. (Consider the message to mean,
"make the state of the file exactly what it claims to be.")
> Yeah. I'm not happy about this comment. Any ideas for a better wording? > ...
11 years, 4 months ago
(2012-11-08 04:56:28 UTC)
#7
> Yeah. I'm not happy about this comment. Any ideas for a better wording?
> These values are documented in the stat(5) man page:
Just calling it "don't touch" rather than "don't-touch" might improve it a bit.
11 years, 4 months ago
(2012-11-26 17:59:19 UTC)
#10
PTAL
https://codereview.appspot.com/6157045/diff/21001/src/pkg/os/file_plan9.go
File src/pkg/os/file_plan9.go (right):
https://codereview.appspot.com/6157045/diff/21001/src/pkg/os/file_plan9.go#ne...
src/pkg/os/file_plan9.go:181: if err := syscall.Fwstat(f.fd, buf[:]); err != nil
{
On 2012/11/26 16:50:42, rsc wrote:
> This is odd. The buffer size should be just the encoded dir, not the maximum
> length. The kernel is ignoring the rest I guess. The return value from
d.Marshal
> should be assigned to n, and the Fwstat should be passed buf[:n]. Same below,
> about a half dozen times.
I don't know what I was thinking. Thanks.
It works because convM2D(2) will return after
successfully parsing an entire message even if
the buffer length is greater than the message
length.
Issue 6157045: code review 6157045: os: move Plan 9 directory marshaling code to syscall
(Closed)
Created 11 years, 11 months ago by ality
Modified 11 years, 4 months ago
Reviewers:
Base URL:
Comments: 11