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

syscall: Getfsstat() doesn't work on freebsd/amd64 #6588

Closed
gopherbot opened this issue Oct 14, 2013 · 17 comments
Closed

syscall: Getfsstat() doesn't work on freebsd/amd64 #6588

gopherbot opened this issue Oct 14, 2013 · 17 comments
Milestone

Comments

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 14, 2013

by preetam@vividcortex.com:

go version go1.1.2 freebsd/amd64

The following program prints out structs filled with 0s, which shouldn't be the case.
__________________________________________________________
package main

import (
        "fmt"
        "os"
        "syscall"
)

func main() {
        len, err := syscall.Getfsstat(nil, 1)
        if err != nil {
                fmt.Println(err)
                os.Exit(1)
        }
        fmt.Println("Length:", len)
        data := make([]syscall.Statfs_t, len)
        _, err = syscall.Getfsstat(data, 1)
        if err != nil {
                fmt.Println(err)
                os.Exit(1)
        }

        fmt.Println("Data:", data)
}
__________________________________________________________
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 14, 2013

Comment 1:

Labels changed: added os-freebsd.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Oct 15, 2013

Comment 2 by juphoff@vividcortex.com:

Calling C.getfsstat() directly via Cgo exhibits the same behavior--a correct number of
structs are returned, but they're entirely filled with 0x0's.
@davecheney
Copy link
Contributor

@davecheney davecheney commented Oct 16, 2013

Comment 3:

Can you ktrace && kdump both programs and see what they produce ?
This doesn't output anything on darwin either,
% go run getfsstat.go 
Length: 0
Data: []

Status changed to WaitingForReply.

@gopherbot
Copy link
Author

@gopherbot gopherbot commented Oct 16, 2013

Comment 4 by preetam@vividcortex.com:

Attached.

Attachments:

  1. getfsstat.dump (13339 bytes)
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Oct 16, 2013

Comment 5 by preetam@vividcortex.com:

This is for getfsstat.go from the first post.

Attachments:

  1. getfsstat.dump (13339 bytes)
@davecheney
Copy link
Contributor

@davecheney davecheney commented Oct 16, 2013

Comment 6:

4487 getfsstat CALL getfsstat(0xc210047000,0x2,0x1<MNT_WAIT>)
4487 getfsstat RET getfsstat 0
^ we asked for 2 fsstat results, we got 0.
Moving to accepted, something strange is going on here, but I don't know if this can be
prioritized for Go 1.2

Labels changed: added os-macosx.

Status changed to Accepted.

@minux
Copy link
Member

@minux minux commented Oct 16, 2013

Comment 7:

the first length = 0 is because we used the wrong pointer.
diff -r 050f1f96c25c src/pkg/syscall/zsyscall_darwin_amd64.go
--- a/src/pkg/syscall/zsyscall_darwin_amd64.go  Tue Oct 15 21:35:52 2013 -0400
+++ b/src/pkg/syscall/zsyscall_darwin_amd64.go  Wed Oct 16 00:41:03 2013 -0400
@@ -594,8 +594,6 @@
    var _p0 unsafe.Pointer
    if len(buf) > 0 {
        _p0 = unsafe.Pointer(&buf[0])
-   } else {
-       _p0 = unsafe.Pointer(&_zero)
    }
    r0, _, e1 := Syscall(SYS_GETFSSTAT64, uintptr(_p0), uintptr(len(buf)), uintptr(flags))
    n = int(r0)
the kernel is not checking nr, but whether _p0 is nil or not to determine whether to
return
the length. as can be seen from the above code snippet, we're always passing a non-nil
pointer to getfsstat.
I'm still not sure about the 2nd problem of returning all zero struct.
This is not a very big deal though, as the user could workaround by using syscall.Syscall
directly (just make sure to use SYS_GETFSSTAT64). Defer to Go 1.3.
Another reason is that although I gave a simple patch above, but there is still a long
way
to go before I can make a CL (we need to fix the program that generates the zsyscall
file,
and i think we'd need a general mechanism to annotate that certain parameter is allowed
to be nil, and then double check all syscall wrappers for the same problem; it will be a
somewhat big project).

Labels changed: added priority-later, go1.3, removed priority-triage.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 8:

Labels changed: added release-go1.3.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 9:

Labels changed: removed go1.3.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 10:

Labels changed: added repo-main.

@ebfe
Copy link
Contributor

@ebfe ebfe commented Jan 13, 2014

Comment 11:

From getfsstat(2):
int getfsstat(struct statfs *buf, long bufsize, int flags);
[...] Note, the bufsize argument is the number of bytes that buf can hold, not the count
of statfs structures it will hold. [...]
syscall.Getfsstat() passes len(buf) as bufsize which is probably < sizeof(struct
statfs) which explains why it doesn't touch buf at all.
@ebfe
Copy link
Contributor

@ebfe ebfe commented Jan 13, 2014

Comment 12:

Passing unsafe.Sizeof(syscall.Statfs_t{}) * uintptr(len(buf)) as bufsize works as
expected (freebsd/386).
http://play.golang.org/p/ypiGCqFTn0
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Mar 25, 2014

Comment 13 by pj@isomero.us:

Could we just create a different function definition (and exclude it from being
automatically generated) instead of modifying the script to handle this case?
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Mar 25, 2014

Comment 14 by preetam@vividcortex.com:

Could we just create a different function definition (and exclude it from being
automatically generated) instead of modifying the script to handle this case?
@rsc
Copy link
Contributor

@rsc rsc commented Apr 3, 2014

Comment 15:

Please just don't use the generator to generate this function body. Add one written by
hand.
@gopherbot
Copy link
Author

@gopherbot gopherbot commented Apr 9, 2014

Comment 16:

CL https://golang.org/cl/84830043 references this issue.
@adg
Copy link
Contributor

@adg adg commented Apr 10, 2014

Comment 17:

This issue was closed by revision 4abbd4a.

Status changed to Fixed.

@rsc rsc added this to the Go1.3 milestone Apr 14, 2015
@rsc rsc removed the release-go1.3 label Apr 14, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
This issue was closed.
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
7 participants
You can’t perform that action at this time.