[PATCH] Re: `pwd -P` with systemd-homed causes inconsistent cwd state

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Sun, Oct 22, 2023 at 11:59 AM Bart Schaefer
<schaefer@xxxxxxxxxxxxxxxx> wrote:
>
> So this reaches the "return NULL" at the end of zgetdir(), after the
> comment "Something bad happened." ?
>
> Or conversely this is a consequence of zgetdir() not checking the
> return value from zchdir(), which it doesn't in a couple of places?

Lacking a response to this and unable to test this directly, I offer
the following patch, which attempts to cover all those cases.

I forced HAVE_GETCWD to be false and ran make check with no issues.

diff --git a/Src/compat.c b/Src/compat.c
index 817bb4aaf..fbed7d80f 100644
--- a/Src/compat.c
+++ b/Src/compat.c
@@ -403,12 +403,25 @@ zgetdir(struct dirsav *d)
 		buf[--pos] = '/';
 	    if (d) {
 #ifndef HAVE_FCHDIR
-		zchdir(buf + pos);
+		if (zchdir(buf + pos) < 0) {
+		    /*
+		     * The reason for all this chdir-ing is to get the
+		     * absolute name of the current directory, so if we
+		     * cannot chdir to that name we have to assume our
+		     * directory is lost.  See "something bad" below.
+		     */
+		    noholdintr();
+		    return NULL;
+		}
 		noholdintr();
 #endif
 		return d->dirname = ztrdup(buf + pos);
 	    }
-	    zchdir(buf + pos);
+	    if (zchdir(buf + pos) < 0) {
+		/* Current directory lost */
+		noholdintr();
+		return NULL;
+	    }
 	    noholdintr();
 	    return buf + pos;
 	}
@@ -477,14 +490,22 @@ zgetdir(struct dirsav *d)
     if (d) {
 #ifndef HAVE_FCHDIR
 	if (buf[pos])
-	    zchdir(buf + pos + 1);
+	    if (zchdir(buf + pos + 1) < 0) {
+		/* Current directory lost */
+		noholdintr();
+		return NULL;
+	    }
 	noholdintr();
 #endif
 	return NULL;
     }
 
     if (buf[pos])
-	zchdir(buf + pos + 1);
+	if (zchdir(buf + pos + 1) < 0) {
+	    /* Current directory lost */
+	    noholdintr();
+	    return NULL;
+	}
     noholdintr();
 
 #else  /* __CYGWIN__, USE_GETCWD cases */
@@ -544,7 +565,11 @@ zgetcwd(void)
     }
 #endif /* HAVE_GETCWD */
     if (!ret)
-	ret = unmeta(pwd);
+	if (chdir(ret = unmeta(pwd)) < 0) {
+	    zwarn("%e: current directory %s lost, using /",
+		  errno, ret);
+	    chdir(ret = dupstring("/"));
+	}
     if (!ret || *ret == '\0')
 	ret = dupstring(".");
     return ret;