I don't really care that much for this idea but it looks fine aside from
minor man page comments inline below.

On Sun, Jul 10, 2011 at 02:24:47PM +0000, Jona Joachim wrote:
> Hi,
> I propose this patch to make realpath(3) conform to SUSv4. It is
> somewhat inspired by FreeBSD.
> When the second argument "resolved" of realpath(3) is set to NULL, the
> behaviour is "implementation-defined" according to IEEE Std 1003.1-2004[1],
> however IEEE Std 1003.1-2008 specifies that realpath(3) should allocate
> memory in that case[2].
> Currently, in OpenBSD, a program segfaults if it calls realpath with
> resolved set to NULL.
> I also tweaked some errno values to comply with SUSv4.
> The standard also says that the C99 "restrict" keyword should be used
> in the prototype of the function but I don't know if this breaks some
> old platforms.
> 
> 1: http://pubs.opengroup.org/onlinepubs/009695399/functions/realpath.html
> 2: http://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html
> 
> (Please bear with me as this is my first time in libc and mdoc)
> 
> Best regards,
> Jona
> 
> 
> Index: realpath.3
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdlib/realpath.3,v
> retrieving revision 1.15
> diff -u -p -r1.15 realpath.3
> --- realpath.3        6 Jul 2007 15:42:04 -0000       1.15
> +++ realpath.3        10 Jul 2011 13:55:40 -0000
> @@ -30,7 +30,7 @@
>  .\"
>  .\"  $OpenBSD: realpath.3,v 1.15 2007/07/06 15:42:04 millert Exp $
>  .\"
> -.Dd $Mdocdate: July 6 2007 $
> +.Dd $Mdocdate: July 10 2011 $
>  .Dt REALPATH 3
>  .Os
>  .Sh NAME
> @@ -40,7 +40,7 @@
>  .Fd #include <limits.h>
>  .Fd #include <stdlib.h>
>  .Ft "char *"
> -.Fn realpath "const char *pathname" "char resolved[PATH_MAX]"
> +.Fn realpath "const char *pathname" "char *resolved"
>  .Sh DESCRIPTION
>  The
>  .Fn realpath
> @@ -60,7 +60,8 @@ argument
>  .Em must
>  refer to a buffer capable of storing at least
>  .Dv PATH_MAX
> -characters.
> +characters, or be
> +.Dv NULL .
>  .Pp
>  The
>  .Fn realpath
> @@ -78,6 +79,13 @@ The
>  function returns
>  .Fa resolved
>  on success.
> +If
> +.Dv NULL
> +was provided as 
> +.Fa resolved
> +and no error was caused, then

How about "If resolved is NULL and no error occurs, then ..."


> +.Fa realpath
> +returns a null-terminated string in a newly allocated buffer.

Strings are NUL-terminated not null-terminated.

>  If an error occurs,
>  .Fn realpath
>  returns
> @@ -98,6 +106,11 @@ and
>  .Sh SEE ALSO
>  .Xr readlink 1 ,
>  .Xr getcwd 3
> +.Sh STANDARDS
> +The
> +.Fn realpath
> +function conforms to
> +.St -p1003.1-2008 .
>  .Sh HISTORY
>  The
>  .Fn realpath
> Index: realpath.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdlib/realpath.c,v
> retrieving revision 1.13
> diff -u -p -r1.13 realpath.c
> --- realpath.c        8 Aug 2005 08:05:37 -0000       1.13
> +++ realpath.c        10 Jul 2011 13:55:40 -0000
> @@ -43,16 +43,34 @@
>   * in which case the path which caused trouble is left in (resolved).
>   */
>  char *
> -realpath(const char *path, char resolved[PATH_MAX])
> +realpath(const char *path, char *resolved)
>  {
>       struct stat sb;
>       char *p, *q, *s;
>       size_t left_len, resolved_len;
>       unsigned symlinks;
> -     int serrno, slen;
> +     int serrno, slen, mem_allocated;
>       char left[PATH_MAX], next_token[PATH_MAX], symlink[PATH_MAX];
>  
> +     if (path == NULL) {
> +             errno = EINVAL;
> +             return (NULL);
> +     }
> +     if (path[0] == '\0') {
> +             errno = ENOENT;
> +             return (NULL);
> +     }
> +
>       serrno = errno;
> +
> +     if (resolved == NULL) {
> +             resolved = malloc(PATH_MAX);
> +             if (resolved == NULL)
> +                     return (NULL);
> +             mem_allocated = 1;
> +     } else
> +             mem_allocated = 0;
> +
>       symlinks = 0;
>       if (path[0] == '/') {
>               resolved[0] = '/';
> @@ -63,7 +81,10 @@ realpath(const char *path, char resolved
>               left_len = strlcpy(left, path + 1, sizeof(left));
>       } else {
>               if (getcwd(resolved, PATH_MAX) == NULL) {
> -                     strlcpy(resolved, ".", PATH_MAX);
> +                     if (mem_allocated)
> +                             free(resolved);
> +                     else
> +                             strlcpy(resolved, ".", PATH_MAX);
>                       return (NULL);
>               }
>               resolved_len = strlen(resolved);
> @@ -71,7 +92,7 @@ realpath(const char *path, char resolved
>       }
>       if (left_len >= sizeof(left) || resolved_len >= PATH_MAX) {
>               errno = ENAMETOOLONG;
> -             return (NULL);
> +             goto err;
>       }
>  
>       /*
> @@ -86,7 +107,7 @@ realpath(const char *path, char resolved
>               s = p ? p : left + left_len;
>               if (s - left >= sizeof(next_token)) {
>                       errno = ENAMETOOLONG;
> -                     return (NULL);
> +                     goto err;
>               }
>               memcpy(next_token, left, s - left);
>               next_token[s - left] = '\0';
> @@ -96,7 +117,7 @@ realpath(const char *path, char resolved
>               if (resolved[resolved_len - 1] != '/') {
>                       if (resolved_len + 1 >= PATH_MAX) {
>                               errno = ENAMETOOLONG;
> -                             return (NULL);
> +                             goto err;
>                       }
>                       resolved[resolved_len++] = '/';
>                       resolved[resolved_len] = '\0';
> @@ -127,23 +148,23 @@ realpath(const char *path, char resolved
>               resolved_len = strlcat(resolved, next_token, PATH_MAX);
>               if (resolved_len >= PATH_MAX) {
>                       errno = ENAMETOOLONG;
> -                     return (NULL);
> +                     goto err;
>               }
>               if (lstat(resolved, &sb) != 0) {
>                       if (errno == ENOENT && p == NULL) {
>                               errno = serrno;
>                               return (resolved);
>                       }
> -                     return (NULL);
> +                     goto err;
>               }
>               if (S_ISLNK(sb.st_mode)) {
>                       if (symlinks++ > MAXSYMLINKS) {
>                               errno = ELOOP;
> -                             return (NULL);
> +                             goto err;
>                       }
>                       slen = readlink(resolved, symlink, sizeof(symlink) - 1);
>                       if (slen < 0)
> -                             return (NULL);
> +                             goto err;
>                       symlink[slen] = '\0';
>                       if (symlink[0] == '/') {
>                               resolved[1] = 0;
> @@ -165,7 +186,7 @@ realpath(const char *path, char resolved
>                               if (symlink[slen - 1] != '/') {
>                                       if (slen + 1 >= sizeof(symlink)) {
>                                               errno = ENAMETOOLONG;
> -                                             return (NULL);
> +                                             goto err;
>                                       }
>                                       symlink[slen] = '/';
>                                       symlink[slen + 1] = 0;
> @@ -173,7 +194,7 @@ realpath(const char *path, char resolved
>                               left_len = strlcat(symlink, left, sizeof(left));
>                               if (left_len >= sizeof(left)) {
>                                       errno = ENAMETOOLONG;
> -                                     return (NULL);
> +                                     goto err;
>                               }
>                       }
>                       left_len = strlcpy(left, symlink, sizeof(left));
> @@ -187,4 +208,9 @@ realpath(const char *path, char resolved
>       if (resolved_len > 1 && resolved[resolved_len - 1] == '/')
>               resolved[resolved_len - 1] = '\0';
>       return (resolved);
> +
> +err:
> +     if (mem_allocated)
> +             free(resolved);
> +     return (NULL);
>  }

Reply via email to