[PATCH] [drawterm] Add devenv and fix exit status for -G flag

[PATCH] [drawterm] Add devenv and fix exit status for -G flag

[Fazlul Shahriar]

Currently drawterm doesn't exit with a non-zero exit status if
the command run with -G flag fails. This issue came up while
using sr.ht CI (see https://todo.sr.ht/~sircmpwn/builds.sr.ht/287).

This problem does not exist in rcpu(1), which saves the exit
status of the remote command in /mnt/term/env/rstatus and then
later uses that for its own exit status. However, drawterm
doesn't have devenv.

This patch adds devenv to drawterm and fixes the exit status
issue. Devenv also initializes /mnt/term/env with environment
variables from the client machine (see kern/devenv.c:/^initunix),
which can be useful even if it's not strictly necessary to solve
the issue.


diff -r cdbacd80f61a cpu.c
--- a/cpu.c	Wed May 27 00:21:41 2020 +0200
+++ b/cpu.c	Fri Jun 19 13:22:19 2020 -0400
@@ -133,6 +133,7 @@
 "	bind -q /mnt/term/dev/cons /dev/cons\n"
 "}\n"
 "</dev/cons >/dev/cons >[2=1] service=cpu %s\n"
+"echo -n $status >/mnt/term/env/rstatus >[2]/dev/null\n"
 "echo -n hangup >/proc/$pid/notepg\n";
 	int fd;
 
@@ -152,6 +153,12 @@
 	}
 	memset(secstorebuf, 0, sizeof(secstorebuf));	/* forget secstore secrets */
 
+	/* $rstatus is used by exportfs to determine exit status */
+	putenv("rstatus=");
+
+	if(bind("#e", "/env", MREPL|MCREATE) < 0)
+		panic("bind #e: %r");
+
 	if(cmd == nil)
 		cmd = smprint(script, "rc -li");
 	else {
diff -r cdbacd80f61a exportfs/exportfs.c
--- a/exportfs/exportfs.c	Wed May 27 00:21:41 2020 +0200
+++ b/exportfs/exportfs.c	Fri Jun 19 13:22:19 2020 -0400
@@ -80,7 +80,7 @@
 		DEBUG(DFD, "read9p...");
 		n = read9pmsg(netfd, r->buf, messagesize);
 		if(n <= 0)
-			fatal(nil);
+			exitrstatus();
 
 		if(convM2S(r->buf, n, &r->work) == 0){
 			iprint("convM2S %d byte message\n", n);
@@ -483,6 +483,23 @@
 	return q;
 }
 
+/*
+ * Exit using the rstatus environment variable, set by the cpu script.
+ */
+void
+exitrstatus()
+{
+	char buf[100];
+	int fd;
+
+	memset(buf, 0, sizeof buf);
+	if((fd = open("/env/rstatus", OREAD)) >= 0){
+		read(fd, buf, (sizeof buf)-1);
+		close(fd);
+	}
+	exits(buf);
+}
+
 void
 fatal(char *s, ...)
 {
diff -r cdbacd80f61a exportfs/exportfs.h
--- a/exportfs/exportfs.h	Wed May 27 00:21:41 2020 +0200
+++ b/exportfs/exportfs.h	Fri Jun 19 13:22:19 2020 -0400
@@ -125,6 +125,7 @@
 Fid	*newfid(int);
 Fsrpc	*getsbuf(void);
 void	initroot(void);
+void	exitrstatus();
 void	fatal(char*, ...);
 char*	makepath(File*, char*);
 File	*file(File*, char*);
diff -r cdbacd80f61a kern/Makefile
--- a/kern/Makefile	Wed May 27 00:21:41 2020 +0200
+++ b/kern/Makefile	Fri Jun 19 13:22:19 2020 -0400
@@ -13,6 +13,7 @@
 	devcmd.$O\
 	devcons.$O\
 	devdraw.$O\
+	devenv.$O\
 	devfs-$(OS).$O\
 	devip.$O\
 	devip-$(OS).$O\
diff -r cdbacd80f61a kern/devenv.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/kern/devenv.c	Fri Jun 19 13:22:19 2020 -0400
@@ -0,0 +1,427 @@
+#include	"u.h"
+#include	"lib.h"
+#include	"dat.h"
+#include	"fns.h"
+#include	"error.h"
+
+enum
+{
+	Maxenvsize = 16300,
+};
+
+static Egrp	*envgrp(Chan *c);
+static int	envwriteable(Chan *c);
+static void	initunix();
+
+static Egrp	unixegrp;	/* unix environment group */
+
+static Evalue*
+envlookup(Egrp *eg, char *name, ulong qidpath)
+{
+	Evalue *e;
+	int i;
+
+	for(i=0; i<eg->nent; i++){
+		e = eg->ent[i];
+		if(e->qid.path == qidpath || (name && e->name[0]==name[0] && strcmp(e->name, name) == 0))
+			return e;
+	}
+	return nil;
+}
+
+static int
+envgen(Chan *c, char *name, Dirtab *_1, int _2, int s, Dir *dp)
+{
+	Egrp *eg;
+	Evalue *e;
+
+	if(s == DEVDOTDOT){
+		devdir(c, c->qid, "#e", 0, eve, DMDIR|0775, dp);
+		return 1;
+	}
+
+	eg = envgrp(c);
+	rlock(&eg->lk);
+	e = 0;
+	if(name)
+		e = envlookup(eg, name, -1);
+	else if(s < eg->nent)
+		e = eg->ent[s];
+
+	if(e == 0) {
+		runlock(&eg->lk);
+		return -1;
+	}
+
+	/* make sure name string continues to exist after we release lock */
+	kstrcpy(up->genbuf, e->name, sizeof up->genbuf);
+	devdir(c, e->qid, up->genbuf, e->len, eve, 0666, dp);
+	runlock(&eg->lk);
+	return 1;
+}
+
+static Chan*
+envattach(char *spec)
+{
+	Chan *c;
+
+	if(spec && *spec) {
+		error(Ebadarg);
+	}
+	initunix();
+	c = devattach('e', spec);
+	c->aux = &unixegrp;
+	return c;
+}
+
+static Walkqid*
+envwalk(Chan *c, Chan *nc, char **name, int nname)
+{
+	return devwalk(c, nc, name, nname, 0, 0, envgen);
+}
+
+static int
+envstat(Chan *c, uchar *db, int n)
+{
+	if(c->qid.type & QTDIR)
+		c->qid.vers = envgrp(c)->vers;
+	return devstat(c, db, n, 0, 0, envgen);
+}
+
+static Chan*
+envopen(Chan *c, int omode)
+{
+	Egrp *eg;
+	Evalue *e;
+	int trunc;
+
+	eg = envgrp(c);
+	if(c->qid.type & QTDIR) {
+		if(omode != OREAD)
+			error(Eperm);
+	}
+	else {
+		trunc = omode & OTRUNC;
+		if(omode != OREAD && !envwriteable(c))
+			error(Eperm);
+		if(trunc)
+			wlock(&eg->lk);
+		else
+			rlock(&eg->lk);
+		e = envlookup(eg, nil, c->qid.path);
+		if(e == 0) {
+			if(trunc)
+				wunlock(&eg->lk);
+			else
+				runlock(&eg->lk);
+			error(Enonexist);
+		}
+		if(trunc && e->value) {
+			e->qid.vers++;
+			free(e->value);
+			e->value = 0;
+			e->len = 0;
+		}
+		if(trunc)
+			wunlock(&eg->lk);
+		else
+			runlock(&eg->lk);
+	}
+	c->mode = openmode(omode);
+	c->flag |= COPEN;
+	c->offset = 0;
+	return c;
+}
+
+static Chan*
+envcreate(Chan *c, char *name, int omode, ulong _)
+{
+	Egrp *eg;
+	Evalue *e;
+	Evalue **ent;
+
+	if(c->qid.type != QTDIR)
+		error(Eperm);
+	if(strlen(name) >= sizeof up->genbuf)
+		error("name too long");			/* protect envgen */
+
+	omode = openmode(omode);
+	eg = envgrp(c);
+
+	wlock(&eg->lk);
+	if(waserror()) {
+		wunlock(&eg->lk);
+		nexterror();
+	}
+
+	if(envlookup(eg, name, -1))
+		error(Eexist);
+
+	e = smalloc(sizeof(Evalue));
+	e->name = smalloc(strlen(name)+1);
+	strcpy(e->name, name);
+
+	if(eg->nent == eg->ment){
+		eg->ment += 32;
+		ent = smalloc(sizeof(eg->ent[0])*eg->ment);
+		if(eg->nent)
+			memmove(ent, eg->ent, sizeof(eg->ent[0])*eg->nent);
+		free(eg->ent);
+		eg->ent = ent;
+	}
+	e->qid.path = ++eg->path;
+	e->qid.vers = 0;
+	eg->vers++;
+	eg->ent[eg->nent++] = e;
+	c->qid = e->qid;
+
+	wunlock(&eg->lk);
+	poperror();
+
+	c->offset = 0;
+	c->mode = omode;
+	c->flag |= COPEN;
+	return c;
+}
+
+static void
+envremove(Chan *c)
+{
+	int i;
+	Egrp *eg;
+	Evalue *e;
+
+	if(c->qid.type & QTDIR)
+		error(Eperm);
+
+	eg = envgrp(c);
+	wlock(&eg->lk);
+	e = 0;
+	for(i=0; i<eg->nent; i++){
+		if(eg->ent[i]->qid.path == c->qid.path){
+			e = eg->ent[i];
+			eg->nent--;
+			eg->ent[i] = eg->ent[eg->nent];
+			eg->vers++;
+			break;
+		}
+	}
+	wunlock(&eg->lk);
+	if(e == 0)
+		error(Enonexist);
+	free(e->name);
+	if(e->value)
+		free(e->value);
+	free(e);
+}
+
+static void
+envclose(Chan *c)
+{
+	/*
+	 * cclose can't fail, so errors from remove will be ignored.
+	 * since permissions aren't checked,
+	 * envremove can't not remove it if its there.
+	 */
+	if(c->flag & CRCLOSE)
+		envremove(c);
+}
+
+static long
+envread(Chan *c, void *a, long n, vlong off)
+{
+	Egrp *eg;
+	Evalue *e;
+	ulong offset = off;
+
+	if(c->qid.type & QTDIR)
+		return devdirread(c, a, n, 0, 0, envgen);
+
+	eg = envgrp(c);
+	rlock(&eg->lk);
+	e = envlookup(eg, nil, c->qid.path);
+	if(e == 0) {
+		runlock(&eg->lk);
+		error(Enonexist);
+	}
+
+	if(offset > e->len)	/* protects against overflow converting vlong to ulong */
+		n = 0;
+	else if(offset + n > e->len)
+		n = e->len - offset;
+	if(n <= 0)
+		n = 0;
+	else
+		memmove(a, e->value+offset, n);
+	runlock(&eg->lk);
+	return n;
+}
+
+static long
+envwrite(Chan *c, void *a, long n, vlong off)
+{
+	char *s;
+	ulong len;
+	Egrp *eg;
+	Evalue *e;
+	ulong offset = off;
+
+	if(n <= 0)
+		return 0;
+	if(offset > Maxenvsize || n > (Maxenvsize - offset))
+		error(Etoobig);
+
+	eg = envgrp(c);
+	wlock(&eg->lk);
+	e = envlookup(eg, nil, c->qid.path);
+	if(e == 0) {
+		wunlock(&eg->lk);
+		error(Enonexist);
+	}
+
+	len = offset+n;
+	if(len > e->len) {
+		s = smalloc(len);
+		if(e->value){
+			memmove(s, e->value, e->len);
+			free(e->value);
+		}
+		e->value = s;
+		e->len = len;
+	}
+	memmove(e->value+offset, a, n);
+	e->qid.vers++;
+	eg->vers++;
+	wunlock(&eg->lk);
+	return n;
+}
+
+Dev envdevtab = {
+	'e',
+	"env",
+
+	devreset,
+	devinit,
+	devshutdown,
+	envattach,
+	envwalk,
+	envstat,
+	envopen,
+	envcreate,
+	envclose,
+	envread,
+	devbread,
+	envwrite,
+	devbwrite,
+	envremove,
+	devwstat,
+};
+
+extern char **environ;
+
+static void
+initunix()
+{
+	Egrp *eg = &unixegrp;
+	Evalue **ent, *e;
+	char *eq, **envp, *line;
+	int n;
+
+	wlock(&eg->lk);
+
+	if(eg->path > 0 || eg->ment > 0 || !environ){
+		// already initialized or nothing in environent
+		wunlock(&eg->lk);
+		return;
+	}
+
+	for(envp = environ; *envp != nil; envp++)
+		eg->ment++;
+	ent = smalloc(sizeof(eg->ent[0])*eg->ment);
+	eg->ent = ent;
+
+	for(envp = environ; *envp != nil; envp++){
+		line = *envp;
+		n = strlen(line);
+
+		eq = strchr(line, '=');
+		if(eq == nil)
+			eq = &line[n];
+		e = smalloc(sizeof(Evalue));
+		e->name = smalloc(eq-line+1);
+		strncpy(e->name, line, eq-line);
+
+
+		if(eq[0] != '\0')
+			eq++;
+		e->len = line+n-eq;
+		e->value = smalloc(e->len);
+		memmove(e->value, eq, e->len);
+
+		e->qid.path = ++eg->path;
+		e->qid.vers = 0;
+		eg->vers++;
+		eg->ent[eg->nent++] = e;
+	}
+
+	wunlock(&eg->lk);
+}
+
+void
+envcpy(Egrp *to, Egrp *from)
+{
+	int i;
+	Evalue *ne, *e;
+
+	rlock(&from->lk);
+	to->ment = (from->nent+31)&~31;
+	to->ent = smalloc(to->ment*sizeof(to->ent[0]));
+	for(i=0; i<from->nent; i++){
+		e = from->ent[i];
+		ne = smalloc(sizeof(Evalue));
+		ne->name = smalloc(strlen(e->name)+1);
+		strcpy(ne->name, e->name);
+		if(e->value){
+			ne->value = smalloc(e->len);
+			memmove(ne->value, e->value, e->len);
+			ne->len = e->len;
+		}
+		ne->qid.path = ++to->path;
+		to->ent[i] = ne;
+	}
+	to->nent = from->nent;
+	runlock(&from->lk);
+}
+
+void
+closeegrp(Egrp *eg)
+{
+	int i;
+	Evalue *e;
+
+	if(decref(&eg->ref) == 0){
+		for(i=0; i<eg->nent; i++){
+			e = eg->ent[i];
+			free(e->name);
+			if(e->value)
+				free(e->value);
+			free(e);
+		}
+		free(eg->ent);
+		free(eg);
+	}
+}
+
+static Egrp*
+envgrp(Chan *c)
+{
+	if(c->aux == nil)
+		return &unixegrp;
+	return c->aux;
+}
+
+static int
+envwriteable(Chan *c)
+{
+	return iseve() || c->aux == nil;
+}
diff -r cdbacd80f61a kern/devtab.c
--- a/kern/devtab.c	Wed May 27 00:21:41 2020 +0200
+++ b/kern/devtab.c	Fri Jun 19 13:22:19 2020 -0400
@@ -18,6 +18,7 @@
 extern Dev audiodevtab;
 extern Dev kbddevtab;
 extern Dev cmddevtab;
+extern Dev envdevtab;
 
 Dev *devtab[] = {
 	&rootdevtab,
@@ -34,6 +35,7 @@
 	&audiodevtab,
 	&kbddevtab,
 	&cmddevtab,
+	&envdevtab,
 	0
 };
 

Re: [9front] [PATCH] [drawterm] Add devenv and fix exit status for -G flag

[cinap_lenrek]

interesting.

i like adding /env to drawterm, however, if we do, we also should import
getenv() and putenv() from plan9 libc too using our /env. (so we can remove
all these #undef getenv lines)

that way we have one unified env and the host specific code will all be
in just one place.

then we need to research if win32 needs any special handling, and possibly
move initunix() to env-win32.c and env-posix.c...

i think the environment in windows also has a unicode version, which needs
a bit of code to convert it to utf-8.

--
cinap

Re: [9front] [PATCH] [drawterm] Add devenv and fix exit status for -G flag

[cinap_lenrek]

just tried to build on windows, and "environ" appears to be there so
everything compiles just fine.

i think we can handle the exitstatus outside of exportfs() just using
atexit() and our libc getenv().

before launching exportfs, we remove /env/rstatus. plan9 getenv()
returns nil when the file doesnt exist in env. so we can distinguish
if the exit status was set by the remote process or not.

Re: [9front] [PATCH] [drawterm] Add devenv and fix exit status for -G flag

[cinap_lenrek]

ok, pushed:

http://code.9front.org/hg/drawterm/rev/8d5dee197436

--
cinap