I don't think this change is OK. If we return NULL we should return the
appropriate errno related to the readlink miss.
There's also another issue here that if we have no trailing slash and
the file it points to doesn't exists it still returns the file as if
it is correct, even though one component pathname doesn't exists:
$ cat realpath.c
#include <limits.h>
#include <stdlib.h>
#include <stdio.h>
int
main(int argc, char *argv[])
{
char *rp;
rp = realpath("/tmp/unexistent", NULL);
printf("%s\n", rp ? rp : "(null)");
}
$ cc ./realpath.c && ./a.out
/tmp/unexistent
$ ls -l /tmp/unexistent
ls: /tmp/unexistent: No such file or directory
My alpine linux box handles this as one would expect.
Both issues were addressed by addressed by doug@ back in r1.17, but
reverted a couple of hours later due to changed semantics found by make
release.
When I find the time I would like to see if the issue back then is
resolved nowadays, so we can retire the entire ENOENT case, or I'll be
happy to give my OK if anyone can confirm this isn't an issue anymore.
While here I would like to address a few nits:
- If realpath fails the content of resolved is undefined, so why waste
cycles setting it to ".". If anyone thinks we should return something
sensible I reckon we should make it an empty string.
- resolved is a char[], so it's a little cleaner to assign it '\0',
we also do this in other parts of the code
- lstat usage was removed by pguenther in r1.21, so remove it from the
manpage.
martijn@
On 06/28/18 04:35, joshua stein wrote:
> realpath(3) on a symlink that points to a non-existent path returns
> that path (but also sets errno), rather than returning NULL and
> setting errno. This is inconsistent with at least Linux, FreeBSD,
> and macOS.
>
> #include <errno.h>
> #include <limits.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
>
> int main()
> {
> char resolved[1024];
> char *ret;
>
> unlink("/tmp/link");
> unlink("/tmp/nonexistent");
> symlink("/tmp/link", "/tmp/nonexistent");
>
> ret = realpath("/tmp/link", resolved);
> if (ret == NULL)
> printf("realpath failed: %s\n", strerror(errno));
> else
> printf("resolved realpath of %s (returned %s), errno is %d\n",
> resolved, ret, errno);
>
> return (0);
> }
>
> Should print:
> realpath failed: No such file or directory
>
> But prints:
> resolved realpath of /tmp/link (returned /tmp/link), errno is 2
>
> This makes it fall through to the error case (in case any memory
> needs to be freed) and return NULL.
>
>
Index: stdlib/realpath.3
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/realpath.3,v
retrieving revision 1.19
diff -u -p -r1.19 realpath.3
--- stdlib/realpath.3 20 Jan 2014 22:40:06 -0000 1.19
+++ stdlib/realpath.3 28 Jun 2018 06:46:00 -0000
@@ -99,8 +99,7 @@ The function
may fail and set the external variable
.Va errno
for any of the errors specified for the library functions
-.Xr lstat 2 ,
-.Xr readlink 2 ,
+.Xr readlink 2
and
.Xr getcwd 3 .
.Sh SEE ALSO
Index: stdlib/realpath.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/realpath.c,v
retrieving revision 1.22
diff -u -p -r1.22 realpath.c
--- stdlib/realpath.c 24 Dec 2017 01:50:50 -0000 1.22
+++ stdlib/realpath.c 28 Jun 2018 06:46:00 -0000
@@ -85,8 +85,6 @@ realpath(const char *path, char *resolve
if (getcwd(resolved, PATH_MAX) == NULL) {
if (mem_allocated)
free(resolved);
- else
- strlcpy(resolved, ".", PATH_MAX);
return (NULL);
}
resolved_len = strlen(resolved);
@@ -185,7 +183,7 @@ realpath(const char *path, char *resolve
symlink[slen] = '\0';
if (symlink[0] == '/') {
- resolved[1] = 0;
+ resolved[1] = '\0';
resolved_len = 1;
} else {
/* Strip the last path component. */
