code review 4440057: os/user: new package to look up users

src/pkg/os/user/lookup_unix.go

 // Copyright 2011 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
 package user
 
 import (
         "fmt"
         "os"
         "runtime"
         "strings"
         "unsafe"
 )
 
 /*
 #include <unistd.h>
 #include <sys/types.h>
 #include <pwd.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);
 }
 */
 import "C"
 
-// LookupId looks up a user by username. If the user isn't found,

[r, 2011/04/21 20:39:32]

s/isn't/cannot be/ here and below

-// os.ENOENT is returned.

[r, 2011/04/21 20:39:32]

is this overspecifying the error return?  we usually don't document so
specifically.

+// Lookup looks up a user by username. If the user cannot be found,
+// the returned error is of type UnknownUserError.
 func Lookup(username string) (*User, os.Error) {
         return lookup(-1, username, true)
 }
 
-// LookupId looks up a user by userid. If the user isn't found,
-// os.ENOENT is returned.
+// LookupId looks up a user by userid. If the user cannot be found,
+// the returned error is of type UnknownUserIdError.
 func LookupId(uid int) (*User, os.Error) {
         return lookup(uid, "", false)
 }
 
 func lookup(uid int, username string, lookupByName bool) (*User, os.Error) {
         var pwd C.struct_passwd
         var result *C.struct_passwd
 
         var bufSize C.long
         if runtime.GOOS == "freebsd" {
                 // FreeBSD doesn't have _SC_GETPW_R_SIZE_MAX
                 // and just returns -1.  So just use the same
                 // size that Linux returns
                 bufSize = 1024
         } else {
                 bufSize = C.sysconf(C._SC_GETPW_R_SIZE_MAX)
-        }
-        if bufSize <= 0 || bufSize > 1<<20 {

[r, 2011/04/21 20:39:32]

this test can go up a line

-                return nil, fmt.Errorf("user: unreasonable _SC_GETPW_R_SIZE_MAX of %d", bufSize)
+                if bufSize <= 0 || bufSize > 1<<20 {
+                        return nil, fmt.Errorf("user: unreasonable _SC_GETPW_R_SIZE_MAX of %d", bufSize)
+                }
         }
         buf := C.malloc(C.size_t(bufSize))
         defer C.free(buf)
         var rv C.int
         if lookupByName {
                 nameC := C.CString(username)
                 defer C.free(unsafe.Pointer(nameC))
                 rv = C.getpwnam_r(nameC,
                         &pwd,
                         (*C.char)(buf),
                         C.size_t(bufSize),
                         &result)
+                if rv != 0 {
+                        return nil, fmt.Errorf("user: lookup username %s: %s", username, os.Errno(rv))
+                }
+                if result == nil {
+                        return nil, UnknownUserError(username)
+                }
         } else {
                 // mygetpwuid_r is a wrapper around getpwuid_r to
                 // to avoid using uid_t because C.uid_t(uid) for
                 // unknown reasons doesn't work on linux.
                 rv = C.mygetpwuid_r(C.int(uid),
                         &pwd,
                         (*C.char)(buf),
                         C.size_t(bufSize),
                         &result)
-        }
-        if rv != 0 {
-                return nil, fmt.Errorf("user: getpwuid_r returned %d", rv)
-        }
-        if result == nil {
-                return nil, os.ENOENT
+                if rv != 0 {
+                        return nil, fmt.Errorf("user: lookup userid %d: %s", uid, os.Errno(rv))
+                }
+                if result == nil {
+                        return nil, UnknownUserIdError(uid)
+                }
         }
         u := &User{
-                Uid:           int(pwd.pw_uid),
-                Gid:           int(pwd.pw_gid),
-                Username:      C.GoString(pwd.pw_name),
-                Name:          C.GoString(pwd.pw_gecos),
-                HomeDirectory: C.GoString(pwd.pw_dir),
+                Uid:      int(pwd.pw_uid),
+                Gid:      int(pwd.pw_gid),
+                Username: C.GoString(pwd.pw_name),
+                Name:     C.GoString(pwd.pw_gecos),
+                HomeDir:  C.GoString(pwd.pw_dir),
         }
         // The pw_gecos field isn't quite standardized.  Some docs
-        // says: "It is expected to be a comma separated list of
+        // say: "It is expected to be a comma separated list of
         // personal data where the first item is the full name of the
         // user."
-        if strings.Contains(u.Name, ",") {
-                sp := strings.Split(u.Name, ",", 2)
-                u.Name = sp[0]
+        if i := strings.Index(u.Name, ","); i >= 0 {
+                u.Name = u.Name[:i]
         }
         return u, nil
 }