os/user: allocate buffers in Go memory, not C memory

Since the first implementation of os/user, it's called C malloc
to allocate memory for buffers. However, the buffers are just
used for temporary storage, and we can just a []byte instead.

To make this work without causing cgo pointer errors, we move
the pwd and grp structs into C stack memory, and just return them.
It's OK to store a Go pointer on the C stack temporarily.

Change-Id: I9f8ffb6e51df1e585276c259fe99359d7835df87
Reviewed-on: https://go-review.googlesource.com/c/go/+/449335
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>

diff --git a/src/os/user/cgo_lookup_unix.go b/src/os/user/cgo_lookup_unix.go
index 4f8577bbc95d8c52596313dfd87bff18a36f7a64..8d328a17a8c213ac32dc06ba3d27b96d677dccac 100644 (file)
--- a/src/os/user/cgo_lookup_unix.go
+++ b/src/os/user/cgo_lookup_unix.go
@@ -22,24 +22,36 @@ import (
 #include <grp.h>
 #include <stdlib.h>
 
-static int mygetpwuid_r(int uid, struct passwd *pwd,
-       char *buf, size_t buflen, struct passwd **result) {
-       return getpwuid_r(uid, pwd, buf, buflen, result);
+static struct passwd mygetpwuid_r(int uid, char *buf, size_t buflen, int *found, int *perr) {
+       struct passwd pwd;
+        struct passwd *result;
+        *perr = getpwuid_r(uid, &pwd, buf, buflen, &result);
+        *found = result != NULL;
+        return pwd;
 }
 
-static int mygetpwnam_r(const char *name, struct passwd *pwd,
-       char *buf, size_t buflen, struct passwd **result) {
-       return getpwnam_r(name, pwd, buf, buflen, result);
+static struct passwd mygetpwnam_r(const char *name, char *buf, size_t buflen, int *found, int *perr) {
+       struct passwd pwd;
+        struct passwd *result;
+        *perr = getpwnam_r(name, &pwd, buf, buflen, &result);
+        *found = result != NULL;
+        return pwd;
 }
 
-static int mygetgrgid_r(int gid, struct group *grp,
-       char *buf, size_t buflen, struct group **result) {
- return getgrgid_r(gid, grp, buf, buflen, result);
+static struct group mygetgrgid_r(int gid, char *buf, size_t buflen, int *found, int *perr) {
+       struct group grp;
+        struct group *result;
+        *perr = getgrgid_r(gid, &grp, buf, buflen, &result);
+        *found = result != NULL;
+        return grp;
 }
 
-static int mygetgrnam_r(const char *name, struct group *grp,
-       char *buf, size_t buflen, struct group **result) {
- return getgrnam_r(name, grp, buf, buflen, result);
+static struct group mygetgrnam_r(const char *name, char *buf, size_t buflen, int *found, int *perr) {
+       struct group grp;
+        struct group *result;
+        *perr = getgrnam_r(name, &grp, buf, buflen, &result);
+        *found = result != NULL;
+        return grp;
 }
 */
 import "C"
@@ -50,28 +62,22 @@ func current() (*User, error) {
 
 func lookupUser(username string) (*User, error) {
        var pwd C.struct_passwd
-       var result *C.struct_passwd
+       var found bool
        nameC := make([]byte, len(username)+1)
        copy(nameC, username)
 
-       buf := alloc(userBuffer)
-       defer buf.free()
-
-       err := retryWithBuffer(buf, func() syscall.Errno {
-               // mygetpwnam_r is a wrapper around getpwnam_r to avoid
-               // passing a size_t to getpwnam_r, because for unknown
-               // reasons passing a size_t to getpwnam_r doesn't work on
-               // Solaris.
-               return syscall.Errno(C.mygetpwnam_r((*C.char)(unsafe.Pointer(&nameC[0])),
-                       &pwd,
-                       (*C.char)(buf.ptr),
-                       C.size_t(buf.size),
-                       &result))
+       err := retryWithBuffer(userBuffer, func(buf []byte) syscall.Errno {
+               var cfound, cerr C.int
+               pwd = C.mygetpwnam_r((*C.char)(unsafe.Pointer(&nameC[0])),
+                       (*C.char)(unsafe.Pointer(&buf[0])), C.size_t(len(buf)),
+                       &cfound, &cerr)
+               found = cfound != 0
+               return syscall.Errno(cerr)
        })
        if err != nil {
                return nil, fmt.Errorf("user: lookup username %s: %v", username, err)
        }
-       if result == nil {
+       if !found {
                return nil, UnknownUserError(username)
        }
        return buildUser(&pwd), err
@@ -87,24 +93,20 @@ func lookupUserId(uid string) (*User, error) {
 
 func lookupUnixUid(uid int) (*User, error) {
        var pwd C.struct_passwd
-       var result *C.struct_passwd
-
-       buf := alloc(userBuffer)
-       defer buf.free()
-
-       err := retryWithBuffer(buf, func() syscall.Errno {
-               // mygetpwuid_r is a wrapper around getpwuid_r to avoid using uid_t
-               // because C.uid_t(uid) for unknown reasons doesn't work on linux.
-               return syscall.Errno(C.mygetpwuid_r(C.int(uid),
-                       &pwd,
-                       (*C.char)(buf.ptr),
-                       C.size_t(buf.size),
-                       &result))
+       var found bool
+
+       err := retryWithBuffer(userBuffer, func(buf []byte) syscall.Errno {
+               var cfound, cerr C.int
+               pwd = C.mygetpwuid_r(C.int(uid),
+                       (*C.char)(unsafe.Pointer(&buf[0])), C.size_t(len(buf)),
+                       &cfound, &cerr)
+               found = cfound != 0
+               return syscall.Errno(cerr)
        })
        if err != nil {
                return nil, fmt.Errorf("user: lookup userid %d: %v", uid, err)
        }
-       if result == nil {
+       if !found {
                return nil, UnknownUserIdError(uid)
        }
        return buildUser(&pwd), nil
@@ -128,24 +130,23 @@ func buildUser(pwd *C.struct_passwd) *User {
 
 func lookupGroup(groupname string) (*Group, error) {
        var grp C.struct_group
-       var result *C.struct_group
+       var found bool
 
-       buf := alloc(groupBuffer)
-       defer buf.free()
        cname := make([]byte, len(groupname)+1)
        copy(cname, groupname)
 
-       err := retryWithBuffer(buf, func() syscall.Errno {
-               return syscall.Errno(C.mygetgrnam_r((*C.char)(unsafe.Pointer(&cname[0])),
-                       &grp,
-                       (*C.char)(buf.ptr),
-                       C.size_t(buf.size),
-                       &result))
+       err := retryWithBuffer(groupBuffer, func(buf []byte) syscall.Errno {
+               var cfound, cerr C.int
+               grp = C.mygetgrnam_r((*C.char)(unsafe.Pointer(&cname[0])),
+                       (*C.char)(unsafe.Pointer(&buf[0])), C.size_t(len(buf)),
+                       &cfound, &cerr)
+               found = cfound != 0
+               return syscall.Errno(cerr)
        })
        if err != nil {
                return nil, fmt.Errorf("user: lookup groupname %s: %v", groupname, err)
        }
-       if result == nil {
+       if !found {
                return nil, UnknownGroupError(groupname)
        }
        return buildGroup(&grp), nil
@@ -161,24 +162,20 @@ func lookupGroupId(gid string) (*Group, error) {
 
 func lookupUnixGid(gid int) (*Group, error) {
        var grp C.struct_group
-       var result *C.struct_group
-
-       buf := alloc(groupBuffer)
-       defer buf.free()
-
-       err := retryWithBuffer(buf, func() syscall.Errno {
-               // mygetgrgid_r is a wrapper around getgrgid_r to avoid using gid_t
-               // because C.gid_t(gid) for unknown reasons doesn't work on linux.
-               return syscall.Errno(C.mygetgrgid_r(C.int(gid),
-                       &grp,
-                       (*C.char)(buf.ptr),
-                       C.size_t(buf.size),
-                       &result))
+       var found bool
+
+       err := retryWithBuffer(groupBuffer, func(buf []byte) syscall.Errno {
+               var cfound, cerr C.int
+               grp = C.mygetgrgid_r(C.int(gid),
+                       (*C.char)(unsafe.Pointer(&buf[0])), C.size_t(len(buf)),
+                       &cfound, &cerr)
+               found = cfound != 0
+               return syscall.Errno(cerr)
        })
        if err != nil {
                return nil, fmt.Errorf("user: lookup groupid %d: %v", gid, err)
        }
-       if result == nil {
+       if !found {
                return nil, UnknownGroupIdError(strconv.Itoa(gid))
        }
        return buildGroup(&grp), nil
@@ -214,44 +211,23 @@ func (k bufferKind) initialSize() C.size_t {
        return C.size_t(sz)
 }
 
-type memBuffer struct {
-       ptr  unsafe.Pointer
-       size C.size_t
-}
-
-func alloc(kind bufferKind) *memBuffer {
-       sz := kind.initialSize()
-       return &memBuffer{
-               ptr:  C.malloc(sz),
-               size: sz,
-       }
-}
-
-func (mb *memBuffer) resize(newSize C.size_t) {
-       mb.ptr = C.realloc(mb.ptr, newSize)
-       mb.size = newSize
-}
-
-func (mb *memBuffer) free() {
-       C.free(mb.ptr)
-}
-
 // retryWithBuffer repeatedly calls f(), increasing the size of the
 // buffer each time, until f succeeds, fails with a non-ERANGE error,
 // or the buffer exceeds a reasonable limit.
-func retryWithBuffer(buf *memBuffer, f func() syscall.Errno) error {
+func retryWithBuffer(startSize bufferKind, f func([]byte) syscall.Errno) error {
+       buf := make([]byte, startSize)
        for {
-               errno := f()
+               errno := f(buf)
                if errno == 0 {
                        return nil
                } else if errno != syscall.ERANGE {
                        return errno
                }
-               newSize := buf.size * 2
+               newSize := len(buf) * 2
                if !isSizeReasonable(int64(newSize)) {
                        return fmt.Errorf("internal buffer exceeds %d bytes", maxBufferSize)
                }
-               buf.resize(newSize)
+               buf = make([]byte, newSize)
        }
 }