On Mon, Mar 21, 2016 at 03:31:42PM +0300, Alexei Malinin wrote:
> Hello.
> 
> I'm not sure but it seems to me that there are several missed things:
> - checking path against NULL,
> - setting errno to ENOMEM in case of malloc() failure,
> - clarification in comments.
> 
> 
> -- 
> Alexei Malinin

Some comments inline.
Thanks.
 
> --- src/lib/libc/stdlib/realpath.c.orig Tue Oct 13 23:55:37 2015
> +++ src/lib/libc/stdlib/realpath.c      Mon Mar 21 15:16:52 2016
> @@ -41,8 +41,9 @@
>   * char *realpath(const char *path, char resolved[PATH_MAX]);
>   *
>   * Find the real name of path, by removing all ".", ".." and symlink
> - * components.  Returns (resolved) on success, or (NULL) on failure,
> - * in which case the path which caused trouble is left in (resolved).
> + * components.  Returns (resolved) on success; sets (errno) and returns
> + * (NULL) on failure, in which case the path which caused trouble
> + * is left in (resolved).
>   */
>  char *
>  realpath(const char *path, char *resolved)
> @@ -54,6 +55,11 @@
>         int serrno, slen, mem_allocated;
>         char left[PATH_MAX], next_token[PATH_MAX], symlink[PATH_MAX];
> 
> +       if (path == NULL) {
> +               errno = ENOENT;
> +               return (NULL);
> +       }
> +

according to 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html,
it should be EINVAL instead of ENOENT.

        The realpath() function shall fail if:
                ...
                [EINVAL]
                        The file_name argument is a null pointer.


>         if (path[0] == '\0') {
>                 errno = ENOENT;
>                 return (NULL);
> @@ -63,8 +69,10 @@
> 
>         if (resolved == NULL) {
>                 resolved = malloc(PATH_MAX);
> -               if (resolved == NULL)
> +               if (resolved == NULL) {
> +                       errno = ENOMEM;

it is unneeded here: malloc(3) already set errno to ENOMEM on failure.

>                         return (NULL);
> +               }
>                 mem_allocated = 1;
>         } else
>                 mem_allocated = 0;
> 
> 
> 

-- 
Sebastien Marie

Reply via email to