code review 34850043: syscall: windows: add Statfs with GetDiskFreeSpace/GetD... (issue 34850043)

264 views
Skip to first unread message

sta...@stalkr.net

unread,
Nov 28, 2013, 11:32:09 AM11/28/13
to golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: golang-dev1,

Message:
Hello golan...@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go


Description:
syscall: windows: add Statfs with GetDiskFreeSpace/GetDiskFreeSpaceEx

Please review this at https://codereview.appspot.com/34850043/

Affected files (+98, -0 lines):
M api/next.txt
M src/pkg/syscall/syscall_windows.go
M src/pkg/syscall/zsyscall_windows_386.go
M src/pkg/syscall/zsyscall_windows_amd64.go


Index: api/next.txt
===================================================================
--- a/api/next.txt
+++ b/api/next.txt
@@ -0,0 +1,22 @@
+pkg syscall (windows-386), func GetDiskFreeSpace(*uint16, *uint32,
*uint32, *uint32, *uint32) error
+pkg syscall (windows-386), func GetDiskFreeSpaceEx(*uint16, *uint64,
*uint64, *uint64) error
+pkg syscall (windows-386), func Statfs(string, *Statfs_t) error
+pkg syscall (windows-386), type Statfs_t struct
+pkg syscall (windows-386), type Statfs_t struct, BytesPerSector uint32
+pkg syscall (windows-386), type Statfs_t struct, FreeBytesAvailable uint64
+pkg syscall (windows-386), type Statfs_t struct, NumberOfFreeClusters
uint32
+pkg syscall (windows-386), type Statfs_t struct, SectorsPerCluster uint32
+pkg syscall (windows-386), type Statfs_t struct, TotalNumberOfBytes uint64
+pkg syscall (windows-386), type Statfs_t struct, TotalNumberOfClusters
uint32
+pkg syscall (windows-386), type Statfs_t struct, TotalNumberOfFreeBytes
uint64
+pkg syscall (windows-amd64), func GetDiskFreeSpace(*uint16, *uint32,
*uint32, *uint32, *uint32) error
+pkg syscall (windows-amd64), func GetDiskFreeSpaceEx(*uint16, *uint64,
*uint64, *uint64) error
+pkg syscall (windows-amd64), func Statfs(string, *Statfs_t) error
+pkg syscall (windows-amd64), type Statfs_t struct
+pkg syscall (windows-amd64), type Statfs_t struct, BytesPerSector uint32
+pkg syscall (windows-amd64), type Statfs_t struct, FreeBytesAvailable
uint64
+pkg syscall (windows-amd64), type Statfs_t struct, NumberOfFreeClusters
uint32
+pkg syscall (windows-amd64), type Statfs_t struct, SectorsPerCluster uint32
+pkg syscall (windows-amd64), type Statfs_t struct, TotalNumberOfBytes
uint64
+pkg syscall (windows-amd64), type Statfs_t struct, TotalNumberOfClusters
uint32
+pkg syscall (windows-amd64), type Statfs_t struct, TotalNumberOfFreeBytes
uint64
Index: src/pkg/syscall/syscall_windows.go
===================================================================
--- a/src/pkg/syscall/syscall_windows.go
+++ b/src/pkg/syscall/syscall_windows.go
@@ -203,6 +203,8 @@
//sys GetConsoleMode(console Handle, mode *uint32) (err error) =
kernel32.GetConsoleMode
//sys WriteConsole(console Handle, buf *uint16, towrite uint32, written
*uint32, reserved *byte) (err error) = kernel32.WriteConsoleW
//sys ReadConsole(console Handle, buf *uint16, toread uint32, read
*uint32, inputControl *byte) (err error) = kernel32.ReadConsoleW
+//sys GetDiskFreeSpace(path *uint16, sectorspercluster *uint32,
bytespersector *uint32, freeclusters *uint32, totalclusters *uint32) (err
error) = kernel32.GetDiskFreeSpaceW
+//sys GetDiskFreeSpaceEx(path *uint16, bytesavailable *uint64, totalbytes
*uint64, totalfreebytes *uint64) (err error) = kernel32.GetDiskFreeSpaceExW

// syscall interface implementation for other packages

@@ -512,6 +514,28 @@
return procSetFileCompletionNotificationModes.Find()
}

+type Statfs_t struct {
+ SectorsPerCluster uint32
+ BytesPerSector uint32
+ NumberOfFreeClusters uint32
+ TotalNumberOfClusters uint32
+ FreeBytesAvailable uint64
+ TotalNumberOfBytes uint64
+ TotalNumberOfFreeBytes uint64
+}
+
+func Statfs(path string, buf *Statfs_t) error {
+ p, e := UTF16PtrFromString(path)
+ if e != nil {
+ return e
+ }
+ err := GetDiskFreeSpace(p, &buf.SectorsPerCluster, &buf.BytesPerSector,
&buf.NumberOfFreeClusters, &buf.TotalNumberOfClusters)
+ if err != nil {
+ return err
+ }
+ return GetDiskFreeSpaceEx(p, &buf.FreeBytesAvailable,
&buf.TotalNumberOfBytes, &buf.TotalNumberOfFreeBytes)
+}
+
// net api calls

const socket_error = uintptr(^uint32(0))
Index: src/pkg/syscall/zsyscall_windows_386.go
===================================================================
--- a/src/pkg/syscall/zsyscall_windows_386.go
+++ b/src/pkg/syscall/zsyscall_windows_386.go
@@ -108,6 +108,8 @@
procGetConsoleMode =
modkernel32.NewProc("GetConsoleMode")
procWriteConsoleW =
modkernel32.NewProc("WriteConsoleW")
procReadConsoleW =
modkernel32.NewProc("ReadConsoleW")
+ procGetDiskFreeSpaceW =
modkernel32.NewProc("GetDiskFreeSpaceW")
+ procGetDiskFreeSpaceExW =
modkernel32.NewProc("GetDiskFreeSpaceExW")
procWSAStartup = modws2_32.NewProc("WSAStartup")
procWSACleanup = modws2_32.NewProc("WSACleanup")
procWSAIoctl = modws2_32.NewProc("WSAIoctl")
@@ -1254,6 +1256,30 @@
return
}

+func GetDiskFreeSpace(path *uint16, sectorspercluster *uint32,
bytespersector *uint32, freeclusters *uint32, totalclusters *uint32) (err
error) {
+ r1, _, e1 := Syscall6(procGetDiskFreeSpaceW.Addr(), 5,
uintptr(unsafe.Pointer(path)), uintptr(unsafe.Pointer(sectorspercluster)),
uintptr(unsafe.Pointer(bytespersector)),
uintptr(unsafe.Pointer(freeclusters)),
uintptr(unsafe.Pointer(totalclusters)), 0)
+ if r1 == 0 {
+ if e1 != 0 {
+ err = error(e1)
+ } else {
+ err = EINVAL
+ }
+ }
+ return
+}
+
+func GetDiskFreeSpaceEx(path *uint16, bytesavailable *uint64, totalbytes
*uint64, totalfreebytes *uint64) (err error) {
+ r1, _, e1 := Syscall6(procGetDiskFreeSpaceExW.Addr(), 4,
uintptr(unsafe.Pointer(path)), uintptr(unsafe.Pointer(bytesavailable)),
uintptr(unsafe.Pointer(totalbytes)),
uintptr(unsafe.Pointer(totalfreebytes)), 0, 0)
+ if r1 == 0 {
+ if e1 != 0 {
+ err = error(e1)
+ } else {
+ err = EINVAL
+ }
+ }
+ return
+}
+
func WSAStartup(verreq uint32, data *WSAData) (sockerr error) {
r0, _, _ := Syscall(procWSAStartup.Addr(), 2, uintptr(verreq),
uintptr(unsafe.Pointer(data)), 0)
if r0 != 0 {
Index: src/pkg/syscall/zsyscall_windows_amd64.go
===================================================================
--- a/src/pkg/syscall/zsyscall_windows_amd64.go
+++ b/src/pkg/syscall/zsyscall_windows_amd64.go
@@ -108,6 +108,8 @@
procGetConsoleMode =
modkernel32.NewProc("GetConsoleMode")
procWriteConsoleW =
modkernel32.NewProc("WriteConsoleW")
procReadConsoleW =
modkernel32.NewProc("ReadConsoleW")
+ procGetDiskFreeSpaceW =
modkernel32.NewProc("GetDiskFreeSpaceW")
+ procGetDiskFreeSpaceExW =
modkernel32.NewProc("GetDiskFreeSpaceExW")
procWSAStartup = modws2_32.NewProc("WSAStartup")
procWSACleanup = modws2_32.NewProc("WSACleanup")
procWSAIoctl = modws2_32.NewProc("WSAIoctl")
@@ -1254,6 +1256,30 @@
return
}

+func GetDiskFreeSpace(path *uint16, sectorspercluster *uint32,
bytespersector *uint32, freeclusters *uint32, totalclusters *uint32) (err
error) {
+ r1, _, e1 := Syscall6(procGetDiskFreeSpaceW.Addr(), 5,
uintptr(unsafe.Pointer(path)), uintptr(unsafe.Pointer(sectorspercluster)),
uintptr(unsafe.Pointer(bytespersector)),
uintptr(unsafe.Pointer(freeclusters)),
uintptr(unsafe.Pointer(totalclusters)), 0)
+ if r1 == 0 {
+ if e1 != 0 {
+ err = error(e1)
+ } else {
+ err = EINVAL
+ }
+ }
+ return
+}
+
+func GetDiskFreeSpaceEx(path *uint16, bytesavailable *uint64, totalbytes
*uint64, totalfreebytes *uint64) (err error) {
+ r1, _, e1 := Syscall6(procGetDiskFreeSpaceExW.Addr(), 4,
uintptr(unsafe.Pointer(path)), uintptr(unsafe.Pointer(bytesavailable)),
uintptr(unsafe.Pointer(totalbytes)),
uintptr(unsafe.Pointer(totalfreebytes)), 0, 0)
+ if r1 == 0 {
+ if e1 != 0 {
+ err = error(e1)
+ } else {
+ err = EINVAL
+ }
+ }
+ return
+}
+
func WSAStartup(verreq uint32, data *WSAData) (sockerr error) {
r0, _, _ := Syscall(procWSAStartup.Addr(), 2, uintptr(verreq),
uintptr(unsafe.Pointer(data)), 0)
if r0 != 0 {


Brad Fitzpatrick

unread,
Nov 28, 2013, 11:39:29 AM11/28/13
to sta...@stalkr.net, golang-dev, re...@codereview-hr.appspotmail.com
Don't update api/* files in the same commit as the change that modifies the API (to minimize merge conflicts when cherry-picking).

Otherwise looks good.






--

---You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

StalkR

unread,
Nov 28, 2013, 11:43:03 AM11/28/13
to Brad Fitzpatrick, golang-dev, re...@codereview-hr.appspotmail.com
Thanks for the quick review, reverted api/next.txt.


--
 
---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.

alex.b...@gmail.com

unread,
Nov 28, 2013, 7:52:47 PM11/28/13
to sta...@stalkr.net, golan...@googlegroups.com, brad...@golang.org, re...@codereview-hr.appspotmail.com
And why do we need this new functionality in syscall package?

Alex

https://codereview.appspot.com/34850043/

StalkR

unread,
Nov 28, 2013, 7:59:03 PM11/28/13
to StalkR, golang-dev, Brad Fitzpatrick, Alex Brainman, re...@codereview-hr.appspotmail.com
Linux has syscall.Statfs but I didn't find anything like it for Windows. Did I miss something?
Of course one can do the syscalls manually like in https://gist.github.com/wendal/5319463 but I thought it could be a good candidate for the standard library.


alex.b...@gmail.com

unread,
Nov 28, 2013, 8:27:02 PM11/28/13
to sta...@stalkr.net, golan...@googlegroups.com, brad...@golang.org, re...@codereview-hr.appspotmail.com
On 2013/11/29 00:59:25, stalkr wrote:
> Linux has syscall.Statfs but I didn't find anything like it for
Windows.

Then it should go into different package. syscall package is for
"platform specific code".

> Of course one can do the syscalls manually like in
> https://gist.github.com/wendal/5319463 ...

This looks fine to me. You can even make this code "go gettable", so
others can easily use it too.

> ... but I thought it could be a good
> candidate for the standard library.

I don't think so. I would use direct Windows API, if I need that
information. There is very little this CL adds to that. Sorry.

I am leaving to others to decide what to do here.

Alex

https://codereview.appspot.com/34850043/
Reply all
Reply to author
Forward
0 new messages