[9front] exportfs: fix debug logging

[9front] exportfs: fix debug logging

[Amavect]

[-- Attachment #1: Type: text/plain, Size: 257 bytes --]

All,

Exportfs -d might not be able to create the debug file without
permission. Instead of letting execution continue, patch 1 makes it
fail.
Patch 2 replaces the DFD macro with an int dfd, simplifying code by 3
lines. Depends on patch 1.

Thanks,
Amavect

[-- Attachment #2: create.1.diff --]
[-- Type: text/x-patch, Size: 514 bytes --]

--- //.git/fs/object/bf6769d3f09d51ff1096afa664c10d3313341a75/tree/sys/src/cmd/exportfs/exportfs.c
+++ sys/src/cmd/exportfs/exportfs.c
@@ -19,12 +19,12 @@
 main(int argc, char **argv)
 {
 	char *dbfile, *srv, *srvfdfile;
-	int n;
 
 	dbfile = "/tmp/exportdb";
 	srv = nil;
 	srvfd = -1;
 	srvfdfile = nil;
+	int n;
 
 	ARGBEGIN{
 	case 'd':
@@ -84,6 +84,8 @@
 
 	if(dbg) {
 		n = create(dbfile, OWRITE|OTRUNC, 0666);
+		if(n < 0)
+			fatal("cannot create debug log: %r", dbfile);
 		dup(n, DFD);
 		close(n);
 	}


[-- Attachment #3: nomacro.2.diff --]
[-- Type: text/x-patch, Size: 8792 bytes --]

--- //.git/fs/object/633dcc8c1cbdc9191e50c3639d8c908fafa32470/tree/sys/src/cmd/exportfs/exportfs.c
+++ sys/src/cmd/exportfs/exportfs.c
@@ -24,7 +24,6 @@
 	srv = nil;
 	srvfd = -1;
 	srvfdfile = nil;
-	int n;
 
 	ARGBEGIN{
 	case 'd':
@@ -83,14 +82,12 @@
 	exclusions();
 
 	if(dbg) {
-		n = create(dbfile, OWRITE|OTRUNC, 0666);
-		if(n < 0)
+		dfd = create(dbfile, OWRITE|OTRUNC, 0666);
+		if(dfd < 0)
 			fatal("cannot create debug log: %r", dbfile);
-		dup(n, DFD);
-		close(n);
 	}
 
-	DEBUG(DFD, "exportfs: started\n");
+	DEBUG(dfd, "exportfs: started\n");
 
 	rfork(RFNOTEG|RFREND);
 
@@ -108,13 +105,13 @@
 			char ebuf[ERRMAX];
 			ebuf[0] = '\0';
 			errstr(ebuf, sizeof ebuf);
-			DEBUG(DFD, "chdir(\"%s\"): %s\n", srv, ebuf);
+			DEBUG(dfd, "chdir(\"%s\"): %s\n", srv, ebuf);
 			mounterror(ebuf);
 		}
-		DEBUG(DFD, "invoked as server for %s", srv);
+		DEBUG(dfd, "invoked as server for %s", srv);
 	}
 
-	DEBUG(DFD, "\niniting root\n");
+	DEBUG(dfd, "\niniting root\n");
 	initroot();
 	io();
 }
--- //.git/fs/object/633dcc8c1cbdc9191e50c3639d8c908fafa32470/tree/sys/src/cmd/exportfs/exportfs.h
+++ sys/src/cmd/exportfs/exportfs.h
@@ -3,7 +3,6 @@
  */
 
 #define DEBUG		if(!dbg){}else fprint
-#define DFD		9
 #define fidhash(s)	fhash[s%FHASHSIZE]
 
 typedef struct Fsrpc Fsrpc;
@@ -86,6 +85,7 @@
 char Enopsmt[];
 
 Extern int  	dbg;
+Extern int	dfd;
 Extern File	*root;
 Extern File	*psmpt;
 Extern Fid	**fhash;
--- //.git/fs/object/633dcc8c1cbdc9191e50c3639d8c908fafa32470/tree/sys/src/cmd/exportfs/exportsrv.c
+++ sys/src/cmd/exportfs/exportsrv.c
@@ -65,7 +65,7 @@
 		w = m->busy;
 		if(w != nil && w->work.tag == t->work.oldtag) {
 			w->flushtag = t->work.tag;
-			DEBUG(DFD, "\tset flushtag %d\n", t->work.tag);
+			DEBUG(dfd, "\tset flushtag %d\n", t->work.tag);
 			postnote(PNPROC, m->pid, "flush");
 			unlock(m);
 			putsbuf(t);
@@ -75,7 +75,7 @@
 	}
 
 	reply(&t->work, &rhdr, 0);
-	DEBUG(DFD, "\tflush reply\n");
+	DEBUG(dfd, "\tflush reply\n");
 	putsbuf(t);
 }
 
@@ -359,7 +359,7 @@
 	}
 
 	path = makepath(f->f, "");
-	DEBUG(DFD, "\tremove: %s\n", path);
+	DEBUG(dfd, "\tremove: %s\n", path);
 	if(remove(path) < 0) {
 		free(path);
 		errstr(err, sizeof err);
@@ -518,7 +518,7 @@
 		if(p == nil)		/* Swept */
 			break;
 
-		DEBUG(DFD, "\tslave: %d %F\n", m->pid, &p->work);
+		DEBUG(dfd, "\tslave: %d %F\n", m->pid, &p->work);
 		if(p->flushtag != NOTAG)
 			goto flushme;
 
@@ -629,7 +629,7 @@
 	}
 	
 	path = makepath(f->f, "");
-	DEBUG(DFD, "\topen: %s %d\n", path, work->mode);
+	DEBUG(dfd, "\topen: %s %d\n", path, work->mode);
 	f->fid = open(path, work->mode);
 	free(path);
 	if(f->fid < 0 || (d = dirfstat(f->fid)) == nil) {
@@ -646,7 +646,7 @@
 			goto Error;
 	}
 
-	DEBUG(DFD, "\topen: fd %d\n", f->fid);
+	DEBUG(dfd, "\topen: fd %d\n", f->fid);
 	f->mode = work->mode;
 	f->offset = 0;
 	rhdr.iounit = getiounit(f->fid);
@@ -688,7 +688,7 @@
 		reply(work, &rhdr, err);
 		return;
 	}
-	DEBUG(DFD, "\tread: fd=%d %d bytes\n", f->fid, r);
+	DEBUG(dfd, "\tread: fd=%d %d bytes\n", f->fid, r);
 
 	rhdr.data = data;
 	rhdr.count = r;
@@ -720,7 +720,7 @@
 		return;
 	}
 
-	DEBUG(DFD, "\twrite: %d bytes fd=%d\n", n, f->fid);
+	DEBUG(dfd, "\twrite: %d bytes fd=%d\n", n, f->fid);
 
 	rhdr.count = n;
 	reply(work, &rhdr, 0);
--- //.git/fs/object/633dcc8c1cbdc9191e50c3639d8c908fafa32470/tree/sys/src/cmd/exportfs/io.c
+++ sys/src/cmd/exportfs/io.c
@@ -49,7 +49,7 @@
 		if(convM2S(r->buf, n, &r->work) != n)
 			fatal("convM2S format error");
 
-		DEBUG(DFD, "%F\n", &r->work);
+		DEBUG(dfd, "%F\n", &r->work);
 		(fcalls[r->work.type])(r);
 	}
 }
@@ -69,7 +69,7 @@
 	else 
 		t->type = r->type + 1;
 
-	DEBUG(DFD, "\t%F\n", t);
+	DEBUG(dfd, "\t%F\n", t);
 
 	data = malloc(messagesize);	/* not mallocz; no need to clear */
 	if(data == nil)
@@ -224,7 +224,7 @@
 
 	while(--f->ref == 0){
 		freecnt++;
-		DEBUG(DFD, "free %s\n", f->name);
+		DEBUG(dfd, "free %s\n", f->name);
 		/* delete from parent */
 		parent = f->parent;
 		if(parent->child == f)
@@ -250,7 +250,7 @@
 	char *path;
 	File *f;
 
-	DEBUG(DFD, "\tfile: 0x%p %s name %s\n", parent, parent->name, name);
+	DEBUG(dfd, "\tfile: 0x%p %s name %s\n", parent, parent->name, name);
 
 	path = makepath(parent, name);
 	if(patternfile != nil && excludefile(path)){
@@ -429,17 +429,17 @@
 	}
 	path = d->qid.path;
 	while(qidexists(path)){
-		DEBUG(DFD, "collision on %s\n", d->name);
+		DEBUG(dfd, "collision on %s\n", d->name);
 		/* collision: find a new one */
 		ncollision++;
 		path &= QIDPATH;
 		++newqid;
 		if(newqid >= (1<<16)){
-			DEBUG(DFD, "collision wraparound\n");
+			DEBUG(dfd, "collision wraparound\n");
 			newqid = 1;
 		}
 		path |= newqid<<48;
-		DEBUG(DFD, "assign qid %.16llux\n", path);
+		DEBUG(dfd, "assign qid %.16llux\n", path);
 	}
 	qidcnt++;
 	q = emallocz(sizeof(Qidtab));
@@ -472,7 +472,7 @@
 		postnote(PNPROC, m->pid, "kill");
 
 	if(s != nil) {
-		DEBUG(DFD, "%s\n", buf);
+		DEBUG(dfd, "%s\n", buf);
 		sysfatal("%s", buf);	/* caution: buf could contain '%' */
 	} else
 		exits(nil);
--- //.git/fs/object/633dcc8c1cbdc9191e50c3639d8c908fafa32470/tree/sys/src/cmd/exportfs/oexportfs.c
+++ sys/src/cmd/exportfs/oexportfs.c
@@ -59,7 +59,7 @@
 		strecpy(strrchr(addr, '!'), addr+sizeof(addr), s);
 	}
 
-	DEBUG(DFD, "filter: %s\n", addr);
+	DEBUG(dfd, "filter: %s\n", addr);
 
 	snprint(buf, sizeof(buf), "%s", cmd);
 	argc = tokenize(buf, argv, nelem(argv)-3);
@@ -256,7 +256,7 @@
 
 	if(dbg) {
 		n = create(dbfile, OWRITE|OTRUNC, 0666);
-		dup(n, DFD);
+		dup(n, dfd);
 		close(n);
 	}
 
@@ -265,7 +265,7 @@
 		usage();
 	}
 
-	DEBUG(DFD, "%s: started\n", argv0);
+	DEBUG(dfd, "%s: started\n", argv0);
 
 	rfork(RFNOTEG|RFREND);
 
@@ -289,10 +289,10 @@
 		if(chdir(srv) < 0) {
 			ebuf[0] = '\0';
 			errstr(ebuf, sizeof ebuf);
-			DEBUG(DFD, "chdir(\"%s\"): %s\n", srv, ebuf);
+			DEBUG(dfd, "chdir(\"%s\"): %s\n", srv, ebuf);
 			mounterror(ebuf);
 		}
-		DEBUG(DFD, "invoked as server for %s", srv);
+		DEBUG(dfd, "invoked as server for %s", srv);
 		strncpy(buf, srv, sizeof buf);
 	}
 	else {
@@ -301,7 +301,7 @@
 		if(n < 0) {
 			errstr(buf, sizeof buf);
 			fprint(0, "read(0): %s\n", buf);
-			DEBUG(DFD, "read(0): %s\n", buf);
+			DEBUG(dfd, "read(0): %s\n", buf);
 			exits(buf);
 		}
 		buf[n] = 0;
@@ -308,15 +308,15 @@
 		if(chdir(buf) < 0) {
 			errstr(ebuf, sizeof ebuf);
 			fprint(0, "chdir(%d:\"%s\"): %s\n", n, buf, ebuf);
-			DEBUG(DFD, "chdir(%d:\"%s\"): %s\n", n, buf, ebuf);
+			DEBUG(dfd, "chdir(%d:\"%s\"): %s\n", n, buf, ebuf);
 			exits(ebuf);
 		}
 	}
 
-	DEBUG(DFD, "\niniting root\n");
+	DEBUG(dfd, "\niniting root\n");
 	initroot();
 
-	DEBUG(DFD, "%s: %s\n", argv0, buf);
+	DEBUG(dfd, "%s: %s\n", argv0, buf);
 
 	if(srv == nil && srvfd == -1 && write(0, "OK", 2) != 2)
 		fatal("open ack write");
@@ -436,7 +436,7 @@
 
 		if(convM2S(r->buf, n, &r->work) != n)
 			fatal("convM2S format error");
-		DEBUG(DFD, "%F\n", &r->work);
+		DEBUG(dfd, "%F\n", &r->work);
 		(fcalls[r->work.type])(r);
 	}
 	io();
--- //.git/fs/object/633dcc8c1cbdc9191e50c3639d8c908fafa32470/tree/sys/src/cmd/exportfs/pattern.c
+++ sys/src/cmd/exportfs/pattern.c
@@ -42,7 +42,7 @@
 				if(include == nil)
 					fatal("out of memory");
 			}
-			DEBUG(DFD, "\tinclude %s\n", line+2);
+			DEBUG(dfd, "\tinclude %s\n", line+2);
 			include[ni] = regcomp(line+2);
 			include[++ni] = nil;
 			break;
@@ -53,12 +53,12 @@
 				if(exclude == nil)
 					fatal("out of memory");
 			}
-			DEBUG(DFD, "\texclude %s\n", line+2);
+			DEBUG(dfd, "\texclude %s\n", line+2);
 			exclude[ne] = regcomp(line+2);
 			exclude[++ne] = nil;
 			break;
 		default:
-			DEBUG(DFD, "ignoring pattern %s\n", line);
+			DEBUG(dfd, "ignoring pattern %s\n", line);
 			break;
 		}
 	}
@@ -76,16 +76,16 @@
 	else
 		p = path+1;
 
-	DEBUG(DFD, "checking %s\n", p);
+	DEBUG(dfd, "checking %s\n", p);
 	for(re = include; *re != nil; re++){
 		if(regexec(*re, p, nil, 0) != 1){
-			DEBUG(DFD, "excluded+ %s\n", p);
+			DEBUG(dfd, "excluded+ %s\n", p);
 			return -1;
 		}
 	}
 	for(re = exclude; *re != nil; re++){
 		if(regexec(*re, p, nil, 0) == 1){
-			DEBUG(DFD, "excluded- %s\n", p);
+			DEBUG(dfd, "excluded- %s\n", p);
 			return -1;
 		}
 	}
@@ -98,7 +98,7 @@
 	int r = 0, m;
 	Dir *d;
 
-	DEBUG(DFD, "\tpreaddir n=%d wo=%lld fo=%lld\n", n, offset, f->offset);
+	DEBUG(dfd, "\tpreaddir n=%d wo=%lld fo=%lld\n", n, offset, f->offset);
 	if(offset == 0 && f->offset != 0){
 		if(seek(f->fid, 0, 0) != 0)
 			return -1;
@@ -128,9 +128,9 @@
 			free(p);
 		}
 		m = convD2M(d, data, n);
-		DEBUG(DFD, "\t\tconvD2M %d\n", m);
+		DEBUG(dfd, "\t\tconvD2M %d\n", m);
 		if(m <= BIT16SZ){
-			DEBUG(DFD, "\t\t\tneeded %d\n", GBIT16(data));
+			DEBUG(dfd, "\t\t\tneeded %d\n", GBIT16(data));
 			/* not enough room for full entry; leave for next time */
 			f->cdir--;
 			return r;

[9front] Re: exportfs: fix debug logging

[Amavect]

[-- Attachment #1: Type: text/plain, Size: 47 bytes --]

How about I use git/export...

Thanks,
Amavect

[-- Attachment #2.1: Type: text/plain, Size: 346 bytes --]

from postmaster@1ess:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: application/octet-stream; name=exportfs.1.git
	Content-Transfer-Encoding: base64
	Content-Disposition: attachment; filename=exportfs.1.git

[-- Attachment #2.2: exportfs.1.git.suspect --]
[-- Type: application/octet-stream, Size: 646 bytes --]

From: amavect <amavect@gmail.com>
Date: Fri, 13 Aug 2021 04:35:14 +0000
Subject: [PATCH] exportfs: exit if debug log couldn't be created


exportfs -d silently continues if the user did
not have permission to create the debug log.
Let's call fatal instead.
---
diff 2af46e406bbd443ae10025777247798a685afc3c 33731ec974a7291fc4b21a8f984bfee058849e3f
--- a/sys/src/cmd/exportfs/exportfs.c	Thu Aug 12 20:27:17 2021
+++ b/sys/src/cmd/exportfs/exportfs.c	Thu Aug 12 23:35:14 2021
@@ -84,6 +84,8 @@
 
 	if(dbg) {
 		n = create(dbfile, OWRITE|OTRUNC, 0666);
+		if(n < 0)
+			fatal("cannot create debug log: %r", dbfile);
 		dup(n, DFD);
 		close(n);
 	}

[-- Attachment #3.1: Type: text/plain, Size: 346 bytes --]

from postmaster@1ess:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: application/octet-stream; name=exportfs.2.git
	Content-Transfer-Encoding: base64
	Content-Disposition: attachment; filename=exportfs.2.git

[-- Attachment #3.2: exportfs.2.git.suspect --]
[-- Type: application/octet-stream, Size: 9163 bytes --]

From: amavect <amavect@gmail.com>
Date: Fri, 13 Aug 2021 04:48:26 +0000
Subject: [PATCH] exportfs: replace DFD macro with variable


exportfs uses a macro with a magic fd number for logging.
It then dups the fd from create() over it.
Let's use the fd directly from create() instead.
Lowercase to match variable style.
---
diff 33731ec974a7291fc4b21a8f984bfee058849e3f b5f7a8b0b47d59797b3cb06cf97a2b8e17120949
--- a/sys/src/cmd/exportfs/exportfs.c	Thu Aug 12 23:35:14 2021
+++ b/sys/src/cmd/exportfs/exportfs.c	Thu Aug 12 23:48:26 2021
@@ -19,7 +19,6 @@
 main(int argc, char **argv)
 {
 	char *dbfile, *srv, *srvfdfile;
-	int n;
 
 	dbfile = "/tmp/exportdb";
 	srv = nil;
@@ -83,14 +82,12 @@
 	exclusions();
 
 	if(dbg) {
-		n = create(dbfile, OWRITE|OTRUNC, 0666);
-		if(n < 0)
+		dfd = create(dbfile, OWRITE|OTRUNC, 0666);
+		if(dfd < 0)
 			fatal("cannot create debug log: %r", dbfile);
-		dup(n, DFD);
-		close(n);
 	}
 
-	DEBUG(DFD, "exportfs: started\n");
+	DEBUG(dfd, "exportfs: started\n");
 
 	rfork(RFNOTEG|RFREND);
 
@@ -108,13 +105,13 @@
 			char ebuf[ERRMAX];
 			ebuf[0] = '\0';
 			errstr(ebuf, sizeof ebuf);
-			DEBUG(DFD, "chdir(\"%s\"): %s\n", srv, ebuf);
+			DEBUG(dfd, "chdir(\"%s\"): %s\n", srv, ebuf);
 			mounterror(ebuf);
 		}
-		DEBUG(DFD, "invoked as server for %s", srv);
+		DEBUG(dfd, "invoked as server for %s", srv);
 	}
 
-	DEBUG(DFD, "\niniting root\n");
+	DEBUG(dfd, "\niniting root\n");
 	initroot();
 	io();
 }
--- a/sys/src/cmd/exportfs/exportfs.h	Thu Aug 12 23:35:14 2021
+++ b/sys/src/cmd/exportfs/exportfs.h	Thu Aug 12 23:48:26 2021
@@ -3,7 +3,6 @@
  */
 
 #define DEBUG		if(!dbg){}else fprint
-#define DFD		9
 #define fidhash(s)	fhash[s%FHASHSIZE]
 
 typedef struct Fsrpc Fsrpc;
@@ -86,6 +85,7 @@
 char Enopsmt[];
 
 Extern int  	dbg;
+Extern int	dfd;
 Extern File	*root;
 Extern File	*psmpt;
 Extern Fid	**fhash;
--- a/sys/src/cmd/exportfs/exportsrv.c	Thu Aug 12 23:35:14 2021
+++ b/sys/src/cmd/exportfs/exportsrv.c	Thu Aug 12 23:48:26 2021
@@ -65,7 +65,7 @@
 		w = m->busy;
 		if(w != nil && w->work.tag == t->work.oldtag) {
 			w->flushtag = t->work.tag;
-			DEBUG(DFD, "\tset flushtag %d\n", t->work.tag);
+			DEBUG(dfd, "\tset flushtag %d\n", t->work.tag);
 			postnote(PNPROC, m->pid, "flush");
 			unlock(m);
 			putsbuf(t);
@@ -75,7 +75,7 @@
 	}
 
 	reply(&t->work, &rhdr, 0);
-	DEBUG(DFD, "\tflush reply\n");
+	DEBUG(dfd, "\tflush reply\n");
 	putsbuf(t);
 }
 
@@ -359,7 +359,7 @@
 	}
 
 	path = makepath(f->f, "");
-	DEBUG(DFD, "\tremove: %s\n", path);
+	DEBUG(dfd, "\tremove: %s\n", path);
 	if(remove(path) < 0) {
 		free(path);
 		errstr(err, sizeof err);
@@ -518,7 +518,7 @@
 		if(p == nil)		/* Swept */
 			break;
 
-		DEBUG(DFD, "\tslave: %d %F\n", m->pid, &p->work);
+		DEBUG(dfd, "\tslave: %d %F\n", m->pid, &p->work);
 		if(p->flushtag != NOTAG)
 			goto flushme;
 
@@ -629,7 +629,7 @@
 	}
 	
 	path = makepath(f->f, "");
-	DEBUG(DFD, "\topen: %s %d\n", path, work->mode);
+	DEBUG(dfd, "\topen: %s %d\n", path, work->mode);
 	f->fid = open(path, work->mode);
 	free(path);
 	if(f->fid < 0 || (d = dirfstat(f->fid)) == nil) {
@@ -646,7 +646,7 @@
 			goto Error;
 	}
 
-	DEBUG(DFD, "\topen: fd %d\n", f->fid);
+	DEBUG(dfd, "\topen: fd %d\n", f->fid);
 	f->mode = work->mode;
 	f->offset = 0;
 	rhdr.iounit = getiounit(f->fid);
@@ -688,7 +688,7 @@
 		reply(work, &rhdr, err);
 		return;
 	}
-	DEBUG(DFD, "\tread: fd=%d %d bytes\n", f->fid, r);
+	DEBUG(dfd, "\tread: fd=%d %d bytes\n", f->fid, r);
 
 	rhdr.data = data;
 	rhdr.count = r;
@@ -720,7 +720,7 @@
 		return;
 	}
 
-	DEBUG(DFD, "\twrite: %d bytes fd=%d\n", n, f->fid);
+	DEBUG(dfd, "\twrite: %d bytes fd=%d\n", n, f->fid);
 
 	rhdr.count = n;
 	reply(work, &rhdr, 0);
--- a/sys/src/cmd/exportfs/io.c	Thu Aug 12 23:35:14 2021
+++ b/sys/src/cmd/exportfs/io.c	Thu Aug 12 23:48:26 2021
@@ -49,7 +49,7 @@
 		if(convM2S(r->buf, n, &r->work) != n)
 			fatal("convM2S format error");
 
-		DEBUG(DFD, "%F\n", &r->work);
+		DEBUG(dfd, "%F\n", &r->work);
 		(fcalls[r->work.type])(r);
 	}
 }
@@ -69,7 +69,7 @@
 	else 
 		t->type = r->type + 1;
 
-	DEBUG(DFD, "\t%F\n", t);
+	DEBUG(dfd, "\t%F\n", t);
 
 	data = malloc(messagesize);	/* not mallocz; no need to clear */
 	if(data == nil)
@@ -224,7 +224,7 @@
 
 	while(--f->ref == 0){
 		freecnt++;
-		DEBUG(DFD, "free %s\n", f->name);
+		DEBUG(dfd, "free %s\n", f->name);
 		/* delete from parent */
 		parent = f->parent;
 		if(parent->child == f)
@@ -250,7 +250,7 @@
 	char *path;
 	File *f;
 
-	DEBUG(DFD, "\tfile: 0x%p %s name %s\n", parent, parent->name, name);
+	DEBUG(dfd, "\tfile: 0x%p %s name %s\n", parent, parent->name, name);
 
 	path = makepath(parent, name);
 	if(patternfile != nil && excludefile(path)){
@@ -429,17 +429,17 @@
 	}
 	path = d->qid.path;
 	while(qidexists(path)){
-		DEBUG(DFD, "collision on %s\n", d->name);
+		DEBUG(dfd, "collision on %s\n", d->name);
 		/* collision: find a new one */
 		ncollision++;
 		path &= QIDPATH;
 		++newqid;
 		if(newqid >= (1<<16)){
-			DEBUG(DFD, "collision wraparound\n");
+			DEBUG(dfd, "collision wraparound\n");
 			newqid = 1;
 		}
 		path |= newqid<<48;
-		DEBUG(DFD, "assign qid %.16llux\n", path);
+		DEBUG(dfd, "assign qid %.16llux\n", path);
 	}
 	qidcnt++;
 	q = emallocz(sizeof(Qidtab));
@@ -472,7 +472,7 @@
 		postnote(PNPROC, m->pid, "kill");
 
 	if(s != nil) {
-		DEBUG(DFD, "%s\n", buf);
+		DEBUG(dfd, "%s\n", buf);
 		sysfatal("%s", buf);	/* caution: buf could contain '%' */
 	} else
 		exits(nil);
--- a/sys/src/cmd/exportfs/oexportfs.c	Thu Aug 12 23:35:14 2021
+++ b/sys/src/cmd/exportfs/oexportfs.c	Thu Aug 12 23:48:26 2021
@@ -59,7 +59,7 @@
 		strecpy(strrchr(addr, '!'), addr+sizeof(addr), s);
 	}
 
-	DEBUG(DFD, "filter: %s\n", addr);
+	DEBUG(dfd, "filter: %s\n", addr);
 
 	snprint(buf, sizeof(buf), "%s", cmd);
 	argc = tokenize(buf, argv, nelem(argv)-3);
@@ -256,7 +256,7 @@
 
 	if(dbg) {
 		n = create(dbfile, OWRITE|OTRUNC, 0666);
-		dup(n, DFD);
+		dup(n, dfd);
 		close(n);
 	}
 
@@ -265,7 +265,7 @@
 		usage();
 	}
 
-	DEBUG(DFD, "%s: started\n", argv0);
+	DEBUG(dfd, "%s: started\n", argv0);
 
 	rfork(RFNOTEG|RFREND);
 
@@ -289,10 +289,10 @@
 		if(chdir(srv) < 0) {
 			ebuf[0] = '\0';
 			errstr(ebuf, sizeof ebuf);
-			DEBUG(DFD, "chdir(\"%s\"): %s\n", srv, ebuf);
+			DEBUG(dfd, "chdir(\"%s\"): %s\n", srv, ebuf);
 			mounterror(ebuf);
 		}
-		DEBUG(DFD, "invoked as server for %s", srv);
+		DEBUG(dfd, "invoked as server for %s", srv);
 		strncpy(buf, srv, sizeof buf);
 	}
 	else {
@@ -301,22 +301,22 @@
 		if(n < 0) {
 			errstr(buf, sizeof buf);
 			fprint(0, "read(0): %s\n", buf);
-			DEBUG(DFD, "read(0): %s\n", buf);
+			DEBUG(dfd, "read(0): %s\n", buf);
 			exits(buf);
 		}
 		buf[n] = 0;
 		if(chdir(buf) < 0) {
 			errstr(ebuf, sizeof ebuf);
 			fprint(0, "chdir(%d:\"%s\"): %s\n", n, buf, ebuf);
-			DEBUG(DFD, "chdir(%d:\"%s\"): %s\n", n, buf, ebuf);
+			DEBUG(dfd, "chdir(%d:\"%s\"): %s\n", n, buf, ebuf);
 			exits(ebuf);
 		}
 	}
 
-	DEBUG(DFD, "\niniting root\n");
+	DEBUG(dfd, "\niniting root\n");
 	initroot();
 
-	DEBUG(DFD, "%s: %s\n", argv0, buf);
+	DEBUG(dfd, "%s: %s\n", argv0, buf);
 
 	if(srv == nil && srvfd == -1 && write(0, "OK", 2) != 2)
 		fatal("open ack write");
@@ -436,7 +436,7 @@
 
 		if(convM2S(r->buf, n, &r->work) != n)
 			fatal("convM2S format error");
-		DEBUG(DFD, "%F\n", &r->work);
+		DEBUG(dfd, "%F\n", &r->work);
 		(fcalls[r->work.type])(r);
 	}
 	io();
--- a/sys/src/cmd/exportfs/pattern.c	Thu Aug 12 23:35:14 2021
+++ b/sys/src/cmd/exportfs/pattern.c	Thu Aug 12 23:48:26 2021
@@ -42,7 +42,7 @@
 				if(include == nil)
 					fatal("out of memory");
 			}
-			DEBUG(DFD, "\tinclude %s\n", line+2);
+			DEBUG(dfd, "\tinclude %s\n", line+2);
 			include[ni] = regcomp(line+2);
 			include[++ni] = nil;
 			break;
@@ -53,12 +53,12 @@
 				if(exclude == nil)
 					fatal("out of memory");
 			}
-			DEBUG(DFD, "\texclude %s\n", line+2);
+			DEBUG(dfd, "\texclude %s\n", line+2);
 			exclude[ne] = regcomp(line+2);
 			exclude[++ne] = nil;
 			break;
 		default:
-			DEBUG(DFD, "ignoring pattern %s\n", line);
+			DEBUG(dfd, "ignoring pattern %s\n", line);
 			break;
 		}
 	}
@@ -76,16 +76,16 @@
 	else
 		p = path+1;
 
-	DEBUG(DFD, "checking %s\n", p);
+	DEBUG(dfd, "checking %s\n", p);
 	for(re = include; *re != nil; re++){
 		if(regexec(*re, p, nil, 0) != 1){
-			DEBUG(DFD, "excluded+ %s\n", p);
+			DEBUG(dfd, "excluded+ %s\n", p);
 			return -1;
 		}
 	}
 	for(re = exclude; *re != nil; re++){
 		if(regexec(*re, p, nil, 0) == 1){
-			DEBUG(DFD, "excluded- %s\n", p);
+			DEBUG(dfd, "excluded- %s\n", p);
 			return -1;
 		}
 	}
@@ -98,7 +98,7 @@
 	int r = 0, m;
 	Dir *d;
 
-	DEBUG(DFD, "\tpreaddir n=%d wo=%lld fo=%lld\n", n, offset, f->offset);
+	DEBUG(dfd, "\tpreaddir n=%d wo=%lld fo=%lld\n", n, offset, f->offset);
 	if(offset == 0 && f->offset != 0){
 		if(seek(f->fid, 0, 0) != 0)
 			return -1;
@@ -128,9 +128,9 @@
 			free(p);
 		}
 		m = convD2M(d, data, n);
-		DEBUG(DFD, "\t\tconvD2M %d\n", m);
+		DEBUG(dfd, "\t\tconvD2M %d\n", m);
 		if(m <= BIT16SZ){
-			DEBUG(DFD, "\t\t\tneeded %d\n", GBIT16(data));
+			DEBUG(dfd, "\t\t\tneeded %d\n", GBIT16(data));
 			/* not enough room for full entry; leave for next time */
 			f->cdir--;
 			return r;

Re: [9front] exportfs: fix debug logging

[ori]

Quoth Amavect <amavect@gmail.com>:
> All,
> 
> Exportfs -d might not be able to create the debug file without
> permission. Instead of letting execution continue, patch 1 makes it
> fail.
> Patch 2 replaces the DFD macro with an int dfd, simplifying code by 3
> lines. Depends on patch 1.
> 
> Thanks,
> Amavect
> 

Second patch looks ok. First patch -- do we want to fail?
Also, why not get rid of the debug fd entirely, and either
syslog() or fprint(2, ...)?

Re: [9front] exportfs: fix debug logging

[Amavect]

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

On Fri, 13 Aug 2021 15:11:18 -0400
ori@eigenstate.org wrote:
> Second patch looks ok. First patch -- do we want to fail?
A computer should do what I tell it to do. -d is explicit.

> Also, why not get rid of the debug fd entirely, and either
> syslog() or fprint(2, ...)?
You make a good point. syslog() would be wrong since this is debug
info and not user info (like ssh logins).

Print to stderr is probably the best default for debug info, and can
also be redirected to a file. This obsoletes -f, doesn't care about
creating a debug log file, and rc fails if the file can't be written.

Attached patch is tested with the following:
srvfs -d man /sys/man
srvfs -d man /sys/man >[2]/tmp/dbg
auth/none srvfs -d man /sys/man >[2]/tmp/dbg

auth/none
srvfs -d man /sys/man >[2]/tmp/dbg  # correctly fails

Thanks,
Amavect

[-- Attachment #2.1: Type: text/plain, Size: 310 bytes --]

from postmaster@1ess:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: text/x-patch
	Content-Transfer-Encoding: 7bit
	Content-Disposition: attachment; filename=exportfs.diff

[-- Attachment #2.2: exportfs.diff.suspect --]
[-- Type: application/octet-stream, Size: 10239 bytes --]

From: amavect <amavect@gmail.com>
Date: Sat, 14 Aug 2021 19:50:23 +0000
Subject: [PATCH] exportfs: make -d log to stderr


exportfs -d logs 9p traffic to /tmp/exportdb.
-f allows writing to a different file.
exportfs silently continues if it doesn't have
permissions to create or write to /tmp/exportdb.
These are poor behaviors.

A better default is to write to stderr, since it
is 9P debug info that is better immediately printed,
and not user info that is better handled by syslog().
As a result, -f is obsolete and thus removed.
Redirect responsibility is now on rc.
As a side effect, rc will fail if it doesn't
have permissions to write.

exportfs(4) is updated to reflect all changes
and with a better Synopsis.
---
diff 2af46e406bbd443ae10025777247798a685afc3c 3b8f235550fedee4c4a42a341e993351a76ecf03
--- a/sys/man/4/exportfs	Thu Aug 12 20:27:17 2021
+++ b/sys/man/4/exportfs	Sat Aug 14 14:50:23 2021
@@ -4,7 +4,23 @@
 .SH SYNOPSIS
 .B exportfs
 [
-.I options
+.B -dsR
+]
+[
+.B -m
+.I msize
+]
+[
+.B -r
+.I root
+]
+[
+.B -P
+.I patternfile
+]
+[
+.B -S
+.I srvfile
 ]
 .PP
 .B srvfs
@@ -39,11 +55,8 @@
 .PP
 The options are:
 .TP
-.B -d -f \fIdbgfile
-Log all 9P traffic to
-.I dbgfile
-(default
-.BR /tmp/exportdb ).
+.B -d
+Log all 9P traffic to stderr.
 .TP
 .B -P \fIpatternfile
 Restrict the set of exported files.
--- a/sys/src/cmd/exportfs/exportfs.c	Thu Aug 12 20:27:17 2021
+++ b/sys/src/cmd/exportfs/exportfs.c	Sat Aug 14 14:50:23 2021
@@ -10,18 +10,16 @@
 void
 usage(void)
 {
-	fprint(2, "usage: %s [-dsR] [-f dbgfile] [-m msize] [-r root] "
-		"[-S srvfile] [-P exclusion-file]\n", argv0);
+	fprint(2, "usage: %s [-dsR] [-m msize] [-r root] "
+		"[-P patternfile] [-S srvfile]\n", argv0);
 	fatal("usage");
 }
 
 void
 main(int argc, char **argv)
 {
-	char *dbfile, *srv, *srvfdfile;
-	int n;
+	char *srv, *srvfdfile;
 
-	dbfile = "/tmp/exportdb";
 	srv = nil;
 	srvfd = -1;
 	srvfdfile = nil;
@@ -31,10 +29,6 @@
 		dbg++;
 		break;
 
-	case 'f':
-		dbfile = EARGF(usage());
-		break;
-
 	case 'm':
 		messagesize = strtoul(EARGF(usage()), nil, 0);
 		break;
@@ -82,13 +76,7 @@
 
 	exclusions();
 
-	if(dbg) {
-		n = create(dbfile, OWRITE|OTRUNC, 0666);
-		dup(n, DFD);
-		close(n);
-	}
-
-	DEBUG(DFD, "exportfs: started\n");
+	DEBUG(2, "exportfs: started\n");
 
 	rfork(RFNOTEG|RFREND);
 
@@ -106,13 +94,13 @@
 			char ebuf[ERRMAX];
 			ebuf[0] = '\0';
 			errstr(ebuf, sizeof ebuf);
-			DEBUG(DFD, "chdir(\"%s\"): %s\n", srv, ebuf);
+			DEBUG(2, "chdir(\"%s\"): %s\n", srv, ebuf);
 			mounterror(ebuf);
 		}
-		DEBUG(DFD, "invoked as server for %s", srv);
+		DEBUG(2, "invoked as server for %s", srv);
 	}
 
-	DEBUG(DFD, "\niniting root\n");
+	DEBUG(2, "\niniting root\n");
 	initroot();
 	io();
 }
--- a/sys/src/cmd/exportfs/exportfs.h	Thu Aug 12 20:27:17 2021
+++ b/sys/src/cmd/exportfs/exportfs.h	Sat Aug 14 14:50:23 2021
@@ -3,7 +3,6 @@
  */
 
 #define DEBUG		if(!dbg){}else fprint
-#define DFD		9
 #define fidhash(s)	fhash[s%FHASHSIZE]
 
 typedef struct Fsrpc Fsrpc;
--- a/sys/src/cmd/exportfs/exportsrv.c	Thu Aug 12 20:27:17 2021
+++ b/sys/src/cmd/exportfs/exportsrv.c	Sat Aug 14 14:50:23 2021
@@ -65,7 +65,7 @@
 		w = m->busy;
 		if(w != nil && w->work.tag == t->work.oldtag) {
 			w->flushtag = t->work.tag;
-			DEBUG(DFD, "\tset flushtag %d\n", t->work.tag);
+			DEBUG(2, "\tset flushtag %d\n", t->work.tag);
 			postnote(PNPROC, m->pid, "flush");
 			unlock(m);
 			putsbuf(t);
@@ -75,7 +75,7 @@
 	}
 
 	reply(&t->work, &rhdr, 0);
-	DEBUG(DFD, "\tflush reply\n");
+	DEBUG(2, "\tflush reply\n");
 	putsbuf(t);
 }
 
@@ -359,7 +359,7 @@
 	}
 
 	path = makepath(f->f, "");
-	DEBUG(DFD, "\tremove: %s\n", path);
+	DEBUG(2, "\tremove: %s\n", path);
 	if(remove(path) < 0) {
 		free(path);
 		errstr(err, sizeof err);
@@ -518,7 +518,7 @@
 		if(p == nil)		/* Swept */
 			break;
 
-		DEBUG(DFD, "\tslave: %d %F\n", m->pid, &p->work);
+		DEBUG(2, "\tslave: %d %F\n", m->pid, &p->work);
 		if(p->flushtag != NOTAG)
 			goto flushme;
 
@@ -629,7 +629,7 @@
 	}
 	
 	path = makepath(f->f, "");
-	DEBUG(DFD, "\topen: %s %d\n", path, work->mode);
+	DEBUG(2, "\topen: %s %d\n", path, work->mode);
 	f->fid = open(path, work->mode);
 	free(path);
 	if(f->fid < 0 || (d = dirfstat(f->fid)) == nil) {
@@ -646,7 +646,7 @@
 			goto Error;
 	}
 
-	DEBUG(DFD, "\topen: fd %d\n", f->fid);
+	DEBUG(2, "\topen: fd %d\n", f->fid);
 	f->mode = work->mode;
 	f->offset = 0;
 	rhdr.iounit = getiounit(f->fid);
@@ -688,7 +688,7 @@
 		reply(work, &rhdr, err);
 		return;
 	}
-	DEBUG(DFD, "\tread: fd=%d %d bytes\n", f->fid, r);
+	DEBUG(2, "\tread: fd=%d %d bytes\n", f->fid, r);
 
 	rhdr.data = data;
 	rhdr.count = r;
@@ -720,7 +720,7 @@
 		return;
 	}
 
-	DEBUG(DFD, "\twrite: %d bytes fd=%d\n", n, f->fid);
+	DEBUG(2, "\twrite: %d bytes fd=%d\n", n, f->fid);
 
 	rhdr.count = n;
 	reply(work, &rhdr, 0);
--- a/sys/src/cmd/exportfs/io.c	Thu Aug 12 20:27:17 2021
+++ b/sys/src/cmd/exportfs/io.c	Sat Aug 14 14:50:23 2021
@@ -49,7 +49,7 @@
 		if(convM2S(r->buf, n, &r->work) != n)
 			fatal("convM2S format error");
 
-		DEBUG(DFD, "%F\n", &r->work);
+		DEBUG(2, "%F\n", &r->work);
 		(fcalls[r->work.type])(r);
 	}
 }
@@ -69,7 +69,7 @@
 	else 
 		t->type = r->type + 1;
 
-	DEBUG(DFD, "\t%F\n", t);
+	DEBUG(2, "\t%F\n", t);
 
 	data = malloc(messagesize);	/* not mallocz; no need to clear */
 	if(data == nil)
@@ -224,7 +224,7 @@
 
 	while(--f->ref == 0){
 		freecnt++;
-		DEBUG(DFD, "free %s\n", f->name);
+		DEBUG(2, "free %s\n", f->name);
 		/* delete from parent */
 		parent = f->parent;
 		if(parent->child == f)
@@ -250,7 +250,7 @@
 	char *path;
 	File *f;
 
-	DEBUG(DFD, "\tfile: 0x%p %s name %s\n", parent, parent->name, name);
+	DEBUG(2, "\tfile: 0x%p %s name %s\n", parent, parent->name, name);
 
 	path = makepath(parent, name);
 	if(patternfile != nil && excludefile(path)){
@@ -429,17 +429,17 @@
 	}
 	path = d->qid.path;
 	while(qidexists(path)){
-		DEBUG(DFD, "collision on %s\n", d->name);
+		DEBUG(2, "collision on %s\n", d->name);
 		/* collision: find a new one */
 		ncollision++;
 		path &= QIDPATH;
 		++newqid;
 		if(newqid >= (1<<16)){
-			DEBUG(DFD, "collision wraparound\n");
+			DEBUG(2, "collision wraparound\n");
 			newqid = 1;
 		}
 		path |= newqid<<48;
-		DEBUG(DFD, "assign qid %.16llux\n", path);
+		DEBUG(2, "assign qid %.16llux\n", path);
 	}
 	qidcnt++;
 	q = emallocz(sizeof(Qidtab));
@@ -472,7 +472,7 @@
 		postnote(PNPROC, m->pid, "kill");
 
 	if(s != nil) {
-		DEBUG(DFD, "%s\n", buf);
+		DEBUG(2, "%s\n", buf);
 		sysfatal("%s", buf);	/* caution: buf could contain '%' */
 	} else
 		exits(nil);
--- a/sys/src/cmd/exportfs/oexportfs.c	Thu Aug 12 20:27:17 2021
+++ b/sys/src/cmd/exportfs/oexportfs.c	Sat Aug 14 14:50:23 2021
@@ -59,7 +59,7 @@
 		strecpy(strrchr(addr, '!'), addr+sizeof(addr), s);
 	}
 
-	DEBUG(DFD, "filter: %s\n", addr);
+	DEBUG(2, "filter: %s\n", addr);
 
 	snprint(buf, sizeof(buf), "%s", cmd);
 	argc = tokenize(buf, argv, nelem(argv)-3);
@@ -256,7 +256,7 @@
 
 	if(dbg) {
 		n = create(dbfile, OWRITE|OTRUNC, 0666);
-		dup(n, DFD);
+		dup(n, 2);
 		close(n);
 	}
 
@@ -265,7 +265,7 @@
 		usage();
 	}
 
-	DEBUG(DFD, "%s: started\n", argv0);
+	DEBUG(2, "%s: started\n", argv0);
 
 	rfork(RFNOTEG|RFREND);
 
@@ -289,10 +289,10 @@
 		if(chdir(srv) < 0) {
 			ebuf[0] = '\0';
 			errstr(ebuf, sizeof ebuf);
-			DEBUG(DFD, "chdir(\"%s\"): %s\n", srv, ebuf);
+			DEBUG(2, "chdir(\"%s\"): %s\n", srv, ebuf);
 			mounterror(ebuf);
 		}
-		DEBUG(DFD, "invoked as server for %s", srv);
+		DEBUG(2, "invoked as server for %s", srv);
 		strncpy(buf, srv, sizeof buf);
 	}
 	else {
@@ -301,22 +301,22 @@
 		if(n < 0) {
 			errstr(buf, sizeof buf);
 			fprint(0, "read(0): %s\n", buf);
-			DEBUG(DFD, "read(0): %s\n", buf);
+			DEBUG(2, "read(0): %s\n", buf);
 			exits(buf);
 		}
 		buf[n] = 0;
 		if(chdir(buf) < 0) {
 			errstr(ebuf, sizeof ebuf);
 			fprint(0, "chdir(%d:\"%s\"): %s\n", n, buf, ebuf);
-			DEBUG(DFD, "chdir(%d:\"%s\"): %s\n", n, buf, ebuf);
+			DEBUG(2, "chdir(%d:\"%s\"): %s\n", n, buf, ebuf);
 			exits(ebuf);
 		}
 	}
 
-	DEBUG(DFD, "\niniting root\n");
+	DEBUG(2, "\niniting root\n");
 	initroot();
 
-	DEBUG(DFD, "%s: %s\n", argv0, buf);
+	DEBUG(2, "%s: %s\n", argv0, buf);
 
 	if(srv == nil && srvfd == -1 && write(0, "OK", 2) != 2)
 		fatal("open ack write");
@@ -436,7 +436,7 @@
 
 		if(convM2S(r->buf, n, &r->work) != n)
 			fatal("convM2S format error");
-		DEBUG(DFD, "%F\n", &r->work);
+		DEBUG(2, "%F\n", &r->work);
 		(fcalls[r->work.type])(r);
 	}
 	io();
--- a/sys/src/cmd/exportfs/pattern.c	Thu Aug 12 20:27:17 2021
+++ b/sys/src/cmd/exportfs/pattern.c	Sat Aug 14 14:50:23 2021
@@ -42,7 +42,7 @@
 				if(include == nil)
 					fatal("out of memory");
 			}
-			DEBUG(DFD, "\tinclude %s\n", line+2);
+			DEBUG(2, "\tinclude %s\n", line+2);
 			include[ni] = regcomp(line+2);
 			include[++ni] = nil;
 			break;
@@ -53,12 +53,12 @@
 				if(exclude == nil)
 					fatal("out of memory");
 			}
-			DEBUG(DFD, "\texclude %s\n", line+2);
+			DEBUG(2, "\texclude %s\n", line+2);
 			exclude[ne] = regcomp(line+2);
 			exclude[++ne] = nil;
 			break;
 		default:
-			DEBUG(DFD, "ignoring pattern %s\n", line);
+			DEBUG(2, "ignoring pattern %s\n", line);
 			break;
 		}
 	}
@@ -76,16 +76,16 @@
 	else
 		p = path+1;
 
-	DEBUG(DFD, "checking %s\n", p);
+	DEBUG(2, "checking %s\n", p);
 	for(re = include; *re != nil; re++){
 		if(regexec(*re, p, nil, 0) != 1){
-			DEBUG(DFD, "excluded+ %s\n", p);
+			DEBUG(2, "excluded+ %s\n", p);
 			return -1;
 		}
 	}
 	for(re = exclude; *re != nil; re++){
 		if(regexec(*re, p, nil, 0) == 1){
-			DEBUG(DFD, "excluded- %s\n", p);
+			DEBUG(2, "excluded- %s\n", p);
 			return -1;
 		}
 	}
@@ -98,7 +98,7 @@
 	int r = 0, m;
 	Dir *d;
 
-	DEBUG(DFD, "\tpreaddir n=%d wo=%lld fo=%lld\n", n, offset, f->offset);
+	DEBUG(2, "\tpreaddir n=%d wo=%lld fo=%lld\n", n, offset, f->offset);
 	if(offset == 0 && f->offset != 0){
 		if(seek(f->fid, 0, 0) != 0)
 			return -1;
@@ -128,9 +128,9 @@
 			free(p);
 		}
 		m = convD2M(d, data, n);
-		DEBUG(DFD, "\t\tconvD2M %d\n", m);
+		DEBUG(2, "\t\tconvD2M %d\n", m);
 		if(m <= BIT16SZ){
-			DEBUG(DFD, "\t\t\tneeded %d\n", GBIT16(data));
+			DEBUG(2, "\t\t\tneeded %d\n", GBIT16(data));
 			/* not enough room for full entry; leave for next time */
 			f->cdir--;
 			return r;

Re: [9front] exportfs: fix debug logging

[kvik]

Quoth Amavect <amavect@gmail.com>:
> Print to stderr is probably the best default for debug info, and can
> also be redirected to a file. This obsoletes -f, doesn't care about
> creating a debug log file, and rc fails if the file can't be written.

This is a nice simplification.  Perhaps you could take a chance to
rename the -d flag to -D for consistency with most other file servers.

LGTM otherwise.

Re: [9front] exportfs: fix debug logging

[unobe]

Quoth kvik@a-b.xyz:
> Quoth Amavect <amavect@gmail.com>:
> > Print to stderr is probably the best default for debug info, and can
> > also be redirected to a file. This obsoletes -f, doesn't care about
> > creating a debug log file, and rc fails if the file can't be written.
> 
> This is a nice simplification.  Perhaps you could take a chance to
> rename the -d flag to -D for consistency with most other file servers.

File servers being anything categorized under section 4?  For example,
I just looked at a few ( cfs(4), cifs(4), sshfs(4) ) which all use -d.

Re: [9front] exportfs: fix debug logging

[ori]

Quoth unobe@cpan.org:
> Quoth kvik@a-b.xyz:
> > Quoth Amavect <amavect@gmail.com>:
> > > Print to stderr is probably the best default for debug info, and can
> > > also be redirected to a file. This obsoletes -f, doesn't care about
> > > creating a debug log file, and rc fails if the file can't be written.
> > 
> > This is a nice simplification.  Perhaps you could take a chance to
> > rename the -d flag to -D for consistency with most other file servers.
> 
> File servers being anything categorized under section 4?  For example,
> I just looked at a few ( cfs(4), cifs(4), sshfs(4) ) which all use -d.
> 

Yeah. it's just a convention (though
I often wish that we'd taken a more
out of the way character for this,
like -δ. :/)

Re: [9front] exportfs: fix debug logging

[kvik]

Quoth unobe@cpan.org:
> File servers being anything categorized under section 4?  For example,
> I just looked at a few ( cfs(4), cifs(4), sshfs(4) ) which all use -d.

Yea, consistency is WIP.  Here's what 9p(2) has to say about it:

	By convention, servers written using this library accept the -D
	option to increment chatty9p.

Note that some file servers that support -D don't care to document it
in the manual -- specifically because it's a "well known" convention.
cifs(4) from your list is one example.

I do realize now that exportfs mixes the 9p trace and general debug
output, so maybe -d is more appropriate after all.  Not sure, and I
certainly don't want to bog down this patch submission with my
bikeshed.  We can deal with many consistency issues at some other
time.

Re: [9front] exportfs: fix debug logging

[unobe]

Quoth ori@eigenstate.org:
> Yeah. it's just a convention (though
> I often wish that we'd taken a more
> out of the way character for this,
> like -δ. :/)

+1

Re: [9front] exportfs: fix debug logging

[unobe]

Quoth kvik@a-b.xyz:
> Quoth unobe@cpan.org:
> > File servers being anything categorized under section 4?  For example,
> > I just looked at a few ( cfs(4), cifs(4), sshfs(4) ) which all use -d.
> 
> Yea, consistency is WIP.  Here's what 9p(2) has to say about it:
> 
> 	By convention, servers written using this library accept the -D
> 	option to increment chatty9p.
> 
> Note that some file servers that support -D don't care to document it
> in the manual -- specifically because it's a "well known" convention.
> cifs(4) from your list is one example.
> 
> I do realize now that exportfs mixes the 9p trace and general debug
> output, so maybe -d is more appropriate after all.  Not sure, and I
> certainly don't want to bog down this patch submission with my
> bikeshed.  We can deal with many consistency issues at some other
> time.
> 

Thanks for the info.  I was thinking the discussion of -D/-d was
tangential anyway so I hadn't intended to hold up the patch from being
committed.  But I had a question about the topic, so figured I'd ask.

Re: [9front] exportfs: fix debug logging

[Amavect]

On Sun, 15 Aug 2021 18:29:16 +0200
kvik@a-b.xyz wrote:
> This is a nice simplification.  Perhaps you could take a chance to
> rename the -d flag to -D for consistency with most other file servers.
Let's move renaming -d to -D for another patch. Then we can do it all
at once. In the meantime, I would like my patch committed (please).


On Mon, 16 Aug 2021 15:55:10 +0200
kvik@a-b.xyz wrote:
> I do realize now that exportfs mixes the 9p trace and general debug
> output, so maybe -d is more appropriate after all.
My conception is that -d and -D do the same thing, which is whitebox
debugging both the program's actions and its communications that
trigger those actions.


On Mon, 16 Aug 2021 01:20:39 -0400
ori@eigenstate.org wrote:
> Yeah. it's just a convention (though
> I often wish that we'd taken a more
> out of the way character for this,
> like -δ. :/)
I support this notion, but I don't have any concrete examples of
mnemonics that would be better suited for -d than "debug" or
"directory". -D is simple enough to free -d for more common tasks.
(Alternatively, debug could be a compile flag, but we chose convenience
over needing to recompile.)


Thanks,
Amavect

Re: [9front] exportfs: fix debug logging

[ori]

Quoth Amavect <amavect@gmail.com>:
> Let's move renaming -d to -D for another patch. Then we can do it all
> at once. In the meantime, I would like my patch committed (please).

Yeah. makes sense. Will look over and commit
today after work -- assuming I don't notice
any glaring issues :)

Re: [9front] exportfs: fix debug logging

[Steve Simon]

hi

the convention i understood, and worked to in cifs is that -d is used for 9p chatty debug, while -D is file server specific debug.

i don't usually document these flags in man pages as they are not intended to be of use to casual users, rather they help those working on the code, and thus the “documentation” is in the code.

i do not claim i have been rigorous in this, only that this was my intention when i wrote it.

-Steve

Re: [9front] exportfs: fix debug logging

[ori]

Quoth ori@eigenstate.org:
> Quoth Amavect <amavect@gmail.com>:
> > Let's move renaming -d to -D for another patch. Then we can do it all
> > at once. In the meantime, I would like my patch committed (please).
> 
> Yeah. makes sense. Will look over and commit
> today after work -- assuming I don't notice
> any glaring issues :)
> 

And, done: thanks!

Re: [9front] exportfs: fix debug logging

[ori]

Quoth Amavect <amavect@gmail.com>:
> I support this notion, but I don't have any concrete examples of
> mnemonics that would be better suited for -d than "debug" 
> "directory".

I haven't kept a list of examples. But I do
run into it often enough -- most recently,
adding 'dns' challenge support to acmed.

Re: [9front] exportfs: fix debug logging

[cinap_lenrek]

i think the global picture hasnt been considered here...

sorry, i'v been a bit sick the last 3 days so wasnt very active...

the removal of the -f flag means that we need to go thru all the
programs that exec exportfs and pass the -d or -f flags.

such as srvfs and iostats.

--
cinap

Re: [9front] exportfs: fix debug logging

[k m]

unsubscribe

Re: [9front] exportfs: fix debug logging

[Humm]

>> You most definitely can. What makes you think otherwise?
>
> Because I tried it.  Example: that Abaco patch (attached to the 
> email you are responding to) except take away the { } around int i; 
> Text *status.
>
> Not complaining, just saying.

Plan 9 C works like C99 in this regard: You can mix declarations and 
statements however you want, but you can’t put labels on declarations.  
(In C2x, you can.)

You find declarations at the beginning of compound statements often 
because they couldn’t be mixed with statements in C89.

-- 
Humm

Re: [9front] exportfs: fix debug logging

[Stuart Morrow]

> You most definitely can. What makes you think otherwise?

Because I tried it.  Example: that Abaco patch (attached to the email
you are responding to) except take away the { } around int i; Text
*status.

Not complaining, just saying.

Re: [9front] exportfs: fix debug logging

[kvik]

Quoth Stuart Morrow <morrow.stuart@gmail.com>:
> In Plan 9 C you *can't* put declarations just anywhere, if you wanted
> to.

You most definitely can. What makes you think otherwise?

Re: [9front] exportfs: fix debug logging

[Stuart Morrow]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

On 13/08/2021, unobe@cpan.org <unobe@cpan.org> wrote:
> I know you already created a different patch, but this part of the
> patch looks pointless, but I don't program in C a lot so maybe I can
> learn something new.  I thought declarations generally go together, so
> why move this one declaration down to after initializations?

In Plan 9 C you *can't* put declarations just anywhere, if you wanted
to. Found this out doing a personal Abaco mod, attached for who may
want it.

[-- Attachment #2: abaco --]
[-- Type: application/octet-stream, Size: 1733 bytes --]

rationale for this is, there was going to be a cursor in the status
bar anyway (and it was going to be ugly anyway), so might as well
put it to use

--- //.git/fs/object/2af46e406bbd443ae10025777247798a685afc3c/tree/sys/src/cmd/abaco/html.c
+++ sys/src/cmd/abaco/html.c
@@ -458,7 +458,7 @@
 void
 mouselink(Box *b, Page *p, int but)
 {
-	Runestr rs;
+	Runestr url, base;
 	Anchor *a;
 
 	/* eat mouse */
@@ -477,18 +477,30 @@
 		return;
 
 	p = whichtarget(p, a->target);
-	rs.r = urlcombine(getbase(p), a->href);
-	if(rs.r == nil)
+	base.r = getbase(p);
+	url.r = urlcombine(base.r, a->href);
+	if(url.r == nil)
 		return;
-	rs.nr = runestrlen(rs.r);
+	url.nr = runestrlen(url.r);
 
-	if(but == 1)
-		pageget(p, &rs, nil, HGet, p==&p->w->page);
-	else if(but == 2)
-		textset(&p->w->status, rs.r, rs.nr);
-	else if(but == 3)
-		plumbrunestr(&rs, nil);
-	closerunestr(&rs);
+	switch(but){
+	case 1:
+		pageget(p, &url, nil, HGet, p==&p->w->page);
+		break;
+	case 2: {
+		Text *status = &p->w->status;
+		int i = min(url.nr, runestrdiff(url.r, base.r));
+
+		textset(status, url.r, url.nr);
+		textsetselect(status, i, i);
+		break;
+		}
+	case 3:
+		plumbrunestr(&url, nil);
+		break;
+	}
+	closerunestr(&url);
+	/* and not needed for &base */
 }
 
 static
--- //.git/fs/object/2af46e406bbd443ae10025777247798a685afc3c/tree/sys/src/cmd/abaco/util.c
+++ sys/src/cmd/abaco/util.c
@@ -14,7 +14,7 @@
 #include "dat.h"
 #include "fns.h"
 
-static	Point		prevmouse;
+static	Point	prevmouse;
 static	Window	*mousew;
 
 int
@@ -149,6 +149,16 @@
 	if(n1 != n2)
 		return FALSE;
 	return memcmp(s1, s2, n1*sizeof(Rune)) == 0;
+}
+
+int
+runestrdiff(Rune *s1, Rune *s2)
+{
+	int n = 0;
+
+	while(*s1++ == *s2++)
+		n++;
+	return n;
 }
 
 void

Re: [9front] exportfs: fix debug logging

[Amavect]

On August 13, 2021 1:46:31 AM CDT, unobe@cpan.org wrote:
>I know you already created a different patch, but this part of the
>patch looks pointless, but I don't program in C a lot so maybe I can
>learn something new.  I thought declarations generally go together, so
>why move this one declaration down to after initializations?

My mistake!
I initially had done both changes (which ultimately deletes that int), and then decided to split them. Instead of using git/revert on that file (as I should have), I remade it by hand. I had to add the variable back, and I thoughtlessly stuck it at the end of the initializations.

Anyways, the second set using git/export doesn't have it, which is correct.

Thanks,
Amavect

Re: [9front] exportfs: fix debug logging

[unobe]

Quoth Amavect <amavect@gmail.com>:
> All,
> 
> Exportfs -d might not be able to create the debug file without
> permission. Instead of letting execution continue, patch 1 makes it
> fail.
> ...
> --- //.git/fs/object/bf6769d3f09d51ff1096afa664c10d3313341a75/tree/sys/src/cmd/exportfs/exportfs.c
> +++ sys/src/cmd/exportfs/exportfs.c
> @@ -19,12 +19,12 @@
>  main(int argc, char **argv)
>  {
>  	char *dbfile, *srv, *srvfdfile;
> -	int n;
>  
>  	dbfile = "/tmp/exportdb";
>  	srv = nil;
>  	srvfd = -1;
>  	srvfdfile = nil;
> +	int n;
>  
>  	ARGBEGIN{
>  	case 'd':

I know you already created a different patch, but this part of the
patch looks pointless, but I don't program in C a lot so maybe I can
learn something new.  I thought declarations generally go together, so
why move this one declaration down to after initializations?