GitLab

at 2023-11-26T14:32:34-08:00

Address review comments.

In `os_get_user_homedir`, use a loop to grow the buffer for `getpwnam_r`
if needed.

The API is changed so that we return a string (or NULL) for the home
directory, and a second arg is the return code which is 0 if
getpwnam_r succeeded (returning the home directory or NULL if the name
does not exist).

Update `unix-get-user-homedir` to use the new API and return two
values: the pathname (or nil) and the return code.

@@ -2902,22 +2902,25 @@
  	c-string))
  (defun unix-get-user-homedir (name)
 -  _N"Get the user home directory for user named NAME.  If there's no such
 -  user or if we don't have enough space to store the path, return NIL."
 -  (with-alien ((homedir (array c-call:char 1024)))
 +  _N"Get the user home directory for user named NAME.  Two values are
 +  returned: the pathname of the home directory and a status code.  If
 +  the home directory does not exist NIL is returned.  The status is 0
 +  if no errors occurred.  Otherwise a non-zero value is returned.
 +  Examining errno may give information about what failed."
 +  (with-alien ((status c-call:int))
      (let ((result
              (alien-funcall
               (extern-alien "os_get_user_homedir"
 -                           (function c-call:int
 +                           (function c-call:c-string
                                       c-call:c-string
 -                                     (* char)
 -                                     c-call:int))
 +                                     (* c-call:int)))
               name
 -             (cast homedir (* c-call:char))
 -             1024)))
 -      (when (zerop result)
 -        (pathname
 -         (concatenate 'string
 -                      (cast homedir c-call:c-string)
 -                      "/"))))))
 +             (addr status))))
 +      (if (and (zerop status) result)
 +          (values (pathname
 +                   (concatenate 'string
 +                                result
 +                                "/"))
 +                  status)
 +          (values result status)))))

@@ -1434,7 +1434,10 @@ msgstr ""
  #: src/code/unix.lisp
  msgid ""
 -"Get the user home directory for user named NAME.  If there's no such\n"
 -"  user or if we don't have enough space to store the path, return NIL."
 +"Get the user home directory for user named NAME.  Two values are\n"
 +"  returned: the pathname of the home directory and a status code.  If\n"
 +"  the home directory does not exist NIL is returned.  The status is 0\n"
 +"  if no errors occurred.  Otherwise a non-zero value is returned.\n"
 +"  Examining errno may give information about what failed."
  msgstr ""

@@ -864,43 +864,65 @@ os_software_version(void)
      return result;
+ }
 -int
 -os_get_user_homedir(const char* name, char* homedir, int len)
 +char *
 +os_get_user_homedir(const char* name, int *status)
+ {
 -    int rc;
      int buflen;
      char * buf;
      struct passwd pwd;
      struct passwd *result;
      buflen = sysconf(_SC_GETPW_R_SIZE_MAX);
 +    /*
 +     * If sysconf failed, just try some possibly large enough value
 +     */
 +    if (buflen == -1) {
 +        buflen = 1024;
 +    }
 -    buf = malloc(buflen);
 +    /*
 +     * sysconf may return a value that is not large enough, so start
 +     * with the given value and keep increasing it until we reach some
 +     * upper limit and give up.
 +     */
 +    while (buflen <= (1 << 20)) {
 +        errno = 0;
 +        buf = malloc(buflen);
 -    if (buf == NULL) {
 -        return -1;
 -    }
 +        if (buf == NULL) {
 +            *status = -1;
 +            return NULL;
 +        }
++
 +        *status = getpwnam_r(name, &pwd, buf, buflen, &result);
 -    rc = getpwnam_r(name, &pwd, buf, buflen, &result);
 +        if (*status == 0) {
 +            /*
 +             * Success, or entry was not found.  If found the result
 +             * is not NULL.  Return the result or NULL
 +             */
 +            fprintf(stderr, "dir = %s\n", pwd.pw_dir);
 +            return result ? strdup(pwd.pw_dir) : NULL;
 +        }
 -    if ((rc == 0) && result != NULL) {
          /*
 -         * Found a matching entry.  Copy it to the output buffer if we
 -         * have room.  If not, set code to -1
 +         * Check errno for ERANGE.  If so, the buffer was too small, so grow it.
           */
 -        if (strlen(pwd.pw_dir) < len) {
 -            strcpy(homedir, pwd.pw_dir);
 +        if (errno == ERANGE) {
 +            free(buf);
 +            buflen *= 2;
          } else {
 -            rc = -1;
 +            /*
 +             * Some other error.  Just return NULL
 +             */
 +            return NULL;
+         }
 -    } else {
 -        rc = -1;
+     }
 -    if (buf) {
 -        free(buf);
 -    }
+-
 -    return rc;
 +    /*
 +     * Ran out of space.  Just return NULL and set status to -1.
 +     */
 +    *status = -1;
 +    return NULL;
+ }

@@ -3,7 +3,7 @@
  (in-package "OS-TESTS")
 -(define-test user-homedir
 +(define-test user-homedir.1
    "Test user-homedir"
    (:tag :issues)
    ;; Simple test to see if unix-get-user-homedir returns the expected
@@ -16,9 +16,22 @@
      (let* ((info-dir (unix:user-info-dir user-info))
             (info-name (unix:user-info-name user-info))
             (expected-home-pathname (pathname
 -                                    (concatenate 'string info-dir "/")))
 -           (home-pathname (unix:unix-get-user-homedir info-name)))
 -      (assert-true info-dir)
 -      (assert-true info-name)
 +                                    (concatenate 'string info-dir "/"))))
 +      (multiple-value-bind (home-pathname status)
 +          (unix:unix-get-user-homedir info-name)
 +        (assert-true info-dir)
 +        (assert-true info-name)
 -      (assert-equal home-pathname expected-home-pathname))))
 +        (assert-equal home-pathname expected-home-pathname)
 +        (assert-eql status 0)))))
++
 +(define-test user-homedir.2
 +  "Test user-homedir"
 +  (:tag :issues)
 +  ;; Simple test to see if unix-get-user-homedir returns the expected
 +  ;; value for a user that does not exist.  Well, we assume such a
 +  ;; user doesn't exist.
 +      (multiple-value-bind (home-pathname status)
 +          (unix:unix-get-user-homedir "zotuserunknown")
 +        (assert-eql home-pathname nil)
 +        (assert-eql status 0)))

Raymond Toy pushed to branch issue-269-unix-get-user-homedir at cmucl / cmucl

Commits:

4 changed files:

Changes: