I just ran a make release with the diff below and ran into the following
error:
[snip]
mount /dev/vnd0a /mnt
cp /tmp/destdir//usr/mdec/boot /usr/src/distrib/amd64/ramdisk_cd/obj/boot
strip /usr/src/distrib/amd64/ramdisk_cd/obj/boot
strip -R .comment -R .SUNW_ctf /usr/src/distrib/amd64/ramdisk_cd/obj/boot
installboot -v -r /mnt /dev/rvnd0c /tmp/destdir//usr/mdec/biosboot
/usr/src/distrib/amd64/ramdisk_cd/obj/boot
Using /mnt as root
installing bootstrap on /dev/rvnd0c
using first-stage /tmp/destdir//usr/mdec/biosboot, second-stage
/usr/src/distrib/amd64/ramdisk_cd/obj/boot
installboot: realpath: No such file or directory
*** Error 1 in /usr/src/distrib/amd64/ramdisk_cd (Makefile.inc:50
'miniroot63.fs')
*** Error 1 in /usr/src/distrib/amd64 (<bsd.subdir.mk>:48 'all')
*** Error 1 in /usr/src/distrib (<bsd.subdir.mk>:48 'all')
*** Error 1 in . (Makefile:292 'distrib')
*** Error 1 in . (Makefile:268 'do-release')
*** Error 1 in /usr/src/etc (Makefile:251 'release')
The problem here is that installboot via fileprefix creates the address
"/mnt//boot", which doesn't exist yet, so with the new realpath we fail
miserably.
I'm not too familiar with installboot, so I'm not sure what the desired
behaviour should be. Some options I can think of are:
- Make "make release" create the directories before calling installboot?
- Make installboot create the files before checking with realpath?
- Hoist realpath up to check just root? A quick grep on fileprefix shows
that fileprefix is only called with root as parameter.
- Remove realpath altogether?
But I can be completely off the mark with this one, so it would be
appreciated if someone with more experience in this area could help me
out.
Note that this is just the first error I've encountered because of this
change, there may be more.
martijn@
On 06/28/18 08:52, Martijn van Duren wrote:
> 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@
>
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 22:02:52 -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 22:02:52 -0000
@@ -49,7 +49,7 @@ realpath(const char *path, char *resolve
char *q;
size_t left_len, resolved_len, next_token_len;
unsigned symlinks;
- int serrno, mem_allocated;
+ int mem_allocated;
ssize_t slen;
char left[PATH_MAX], next_token[PATH_MAX], symlink[PATH_MAX];
@@ -63,8 +63,6 @@ realpath(const char *path, char *resolve
return (NULL);
}
- serrno = errno;
-
if (resolved == NULL) {
resolved = malloc(PATH_MAX);
if (resolved == NULL)
@@ -85,8 +83,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);
@@ -162,12 +158,6 @@ realpath(const char *path, char *resolve
case EINVAL:
/* not a symlink, continue to next component */
continue;
- case ENOENT:
- if (p == NULL) {
- errno = serrno;
- return (resolved);
- }
- /* FALLTHROUGH */
default:
goto err;
}
@@ -185,7 +175,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. */
