[PATCH] syscall: utility overhaul

[PATCH] syscall: utility overhaul

[kvik]

Following is a list of functional changes:

* The -o flag outputs the entire buffer to the length returned
  by the syscall, or, in case of fd2path(2) and errstr(2), to '\0'.
* The -x flag is removed; the above makes it possible to pipe
  into xd(1) to get the same result.
* The -s flag uses dirfmt(2) to format the stat message, instead
  of trying to imitate ls(1).
* Stderr reports are normalized and made easier to parse.

The code also suffered a number of stylistic changes.

diff -r d947a4c5d7fa sys/man/1/syscall
--- a/sys/man/1/syscall	Thu Sep 03 20:07:44 2020 -0700
+++ b/sys/man/1/syscall	Sat Sep 19 15:04:17 2020 +0200
@@ -4,7 +4,7 @@
 .SH SYNOPSIS
 .B syscall
 [
-.B -osx
+.B -os
 ]
 .I entry
 [
@@ -15,39 +15,44 @@
 invokes the system call
 .I entry
 with the given arguments.
-(Some functions, such as
-.I write
+The return value is printed.
+If an error occured, the error string is also printed.
+.PP
+For convenience,
+.IR write (2)
 and
-.IR read (2),
-although not strictly system calls, are valid
-.IR entries .)
-It prints the return value and the error string, if there was an error.
-An argument is either an integer constant as in C (its value is passed),
-a string (its address is passed),
-or the literal
+.IR read (2)
+are included in
+.IR entries ,
+even though they are not strictly syscalls.
+.PP
+.I Syscall
+arguments are integer constants, strings, or the literal
+.BR buf .
+The literal
 .B buf
-(a pointer to a 1MB buffer is passed).
+refers to a writable 1 megabyte buffer.
+Strings and 
+.B buf
+are passed as pointers.
+Integers are passed as values.
 .PP
-If
+The
 .B -o
-is given, the contents of the 1MB buffer are printed as a zero-terminated string
-after the system call is done.
+option prints contents of the 1MB buffer.
+For
+.IR errstr (2)
+and
+.IR fd2path (2),
+the buffer is treated as a 0-terminated string.
+For other calls, the number of bytes printed is
+determined by the system call's return value.
+.PP
 The
-.B -x
-and
 .B -s
-options are similar, but
-.B -x
-formats the data as hexadecimal bytes, while
-.B -s
-interprets the data as a
+option is similar, but interprets the data as a
 .IR stat (5)
-message and formats it similar to the style of
-.B ls
-.B -lqm
-(see
-.IR ls (1)),
-with extra detail about the modify and access times.
+message and formats it to standard output.
 .SH EXAMPLES
 Write a string to standard output:
 .IP
diff -r d947a4c5d7fa sys/src/cmd/syscall/mkfile
--- a/sys/src/cmd/syscall/mkfile	Thu Sep 03 20:07:44 2020 -0700
+++ b/sys/src/cmd/syscall/mkfile	Sat Sep 19 15:04:17 2020 +0200
@@ -1,5 +1,4 @@
 </$objtype/mkfile
-CFLAGS=-I/sys/src/libc/9syscall $CFLAGS
 
 TARG=syscall
 OFILES=syscall.$O\
@@ -18,12 +17,24 @@
 SYSCALL=/sys/src/libc/9syscall/sys.h
 
 tab.h:	$SYSCALL
-	sed '/#define._X[0-9_]/d;
-		/#define.NSYSCALL/d;
-		s/#define.([A-Z0-9_][A-Z0-9_]*).*/	"\1",	(int(*)(...))\1,/' $SYSCALL |
-		tr A-Z a-z > tab.h
-	echo '	"read",	(int(*)(...))read,' >> tab.h
-	echo '	"write",	(int(*)(...))write,' >> tab.h
+	awk '
+	BEGIN{ print "enum{" }
+	{ printf "%s, ", $2 }
+	END{
+		print "READ, WRITE, NTAB"
+		print "};"
+	}' <$SYSCALL >$target 
+	awk '
+	BEGIN{ print "struct Call tab[] = {" }
+	{ printf "[%s] \"%s\", (int(*)(...))%s,\n",
+		$2, tolower($2), tolower($2)
+	}
+	END{
+		print "[READ] \"read\", (int(*)(...))read,"
+		print "[WRITE] \"write\", (int(*)(...))write,"
+		print "[NTAB] nil, 0"
+		print "};"
+	}' <$SYSCALL >>$target
 
 clean:V:
 	rm -f *.[$OS] [$OS].out $TARG $HFILES
diff -r d947a4c5d7fa sys/src/cmd/syscall/syscall.c
--- a/sys/src/cmd/syscall/syscall.c	Thu Sep 03 20:07:44 2020 -0700
+++ b/sys/src/cmd/syscall/syscall.c	Sat Sep 19 15:04:17 2020 +0200
@@ -1,10 +1,9 @@
 #include <u.h>
 #include <libc.h>
-#include <sys.h>
 #include <fcall.h>
 
 char	buf[1048576];
-#define	NARG	5
+enum{ NARG = 5 };
 uintptr	arg[NARG];
 
 /* system calls not defined in libc.h */
@@ -28,65 +27,55 @@
 int	_wait(void*);
 int	_nsec(vlong*);
 
-struct{
+struct Call{
 	char	*name;
 	int	(*func)(...);
-}tab[]={
+};
 #include "tab.h"
-	0,		0
-};
 
-uintptr parse(char *);
-void catch(void*, char*);
-
-char*
-xctime(ulong t)
+void
+usage(void)
 {
-	char *buf, *s;
-
-	s = ctime(t);
-	s[strlen(s)-1] = '\0';	/* remove newline */
-	buf = malloc(512);
-	if(buf == nil)
-		sysfatal("can't malloc: %r");
-	snprint(buf, 512, "%s (%lud)", s, t);
-	return buf;
+	fprint(2, "usage: %s [-os] entry [arg ...]\n", argv0);
+	exits("usage");
 }
 
+uintptr
+parse(char *s)
+{
+	char *t;
+	uintptr l;
 
-char*
-lstime(long l)
+	if(strncmp(s, "buf", 3) == 0)
+		return (uintptr)buf;
+	
+	l = strtoull(s, &t, 0);
+	if(t > s && *t == 0)
+		return l;
+
+	return (uintptr)s; 
+}
+
+void
+catch(void *, char *msg)
 {
-	static char buf[32];
-	char *t;
-	long clk;
-
-	clk = time(0);
-	t = ctime(l);
-	/* 6 months in the past or a day in the future */
-	if(l<clk-180L*24*60*60 || clk+24L*60*60<l){
-		memmove(buf, t+4, 7);		/* month and day */
-		memmove(buf+7, t+23, 5);		/* year */
-	}else
-		memmove(buf, t+4, 12);		/* skip day of week */
-	buf[12] = 0;
-	return buf;
+	fprint(2, "syscall: received note: %s\n", msg);
+	noted(NDFLT);
 }
 
 void
 main(int argc, char *argv[])
 {
-	int i, j, c;
-	int oflag, xflag, sflag;
-	vlong r;
+	int i;
+	int oflag, sflag;
+	vlong r, nbuf;
 	Dir d;
 	char strs[1024];
-	char ebuf[1024];
+	char ebuf[ERRMAX];
 
-	fmtinstall('M', dirmodefmt);
+	fmtinstall('D', dirfmt);
 
 	oflag = 0;
-	xflag = 0;
 	sflag = 0;
 	ARGBEGIN{
 	case 'o':
@@ -95,97 +84,57 @@
 	case 's':
 		sflag++;
 		break;
-	case 'x':
-		xflag++;
+	default:
+		usage();
+	}ARGEND
+	if(argc < 1 || argc > 1+NARG)
+		usage();
+
+	for(i = 1; i < argc; i++)
+		arg[i-1] = parse(argv[i]);
+	for(i = 0; tab[i].name; i++)
+		if(strcmp(tab[i].name, argv[0]) == 0)
+			break;
+	if(i == NTAB){
+		fprint(2, "syscall: %s not known\n", argv[0]);
+		exits("unknown");
+	}
+	notify(catch);
+	/* special case for seek, pread, pwrite; vlongs are problematic */
+	switch(i){
+	default:
+		r = (*tab[i].func)(arg[0], arg[1], arg[2], arg[3], arg[4]);
 		break;
-	default:
-		goto Usage;
-	}ARGEND
-	if(argc<1 || argc>1+NARG){
-    Usage:
-		fprint(2, "usage: syscall [-ox] entry [args; buf==1MB buffer]\n");
-		fprint(2, "\tsyscall write 1 hello 5\n");
-		fprint(2, "\tsyscall -o errstr buf 1024\n");
-		fprint(2, "\tsyscall -[xs] stat file buf 1024\n");
-		exits("usage");
+	case SEEK:
+		r = seek(arg[0], strtoll(argv[2], 0, 0), arg[2]);
+		break;
+	case PREAD:
+		r = pread(arg[0], (void*)arg[1], arg[2], strtoll(argv[4], 0, 0));
+		break;
+	case PWRITE:
+		r = pwrite(arg[0], (void*)arg[1], arg[2], strtoll(argv[4], 0, 0));
+		break;
 	}
-	for(i=1; i<argc; i++)
-		arg[i-1] = parse(argv[i]);
-	notify(catch);
-	for(i=0; tab[i].name; i++)
-		if(strcmp(tab[i].name, argv[0])==0){
-			/* special case for seek, pread, pwrite; vlongs are problematic */
-			if(strcmp(argv[0], "seek") == 0)
-				r=seek(arg[0], strtoll(argv[2], 0, 0), arg[2]);
-			else if(strcmp(argv[0], "pread") == 0)
-				r=pread(arg[0], (void*)arg[1], arg[2], strtoll(argv[4], 0, 0));
-			else if(strcmp(argv[0], "pwrite") == 0)
-				r=pwrite(arg[0], (void*)arg[1], arg[2], strtoll(argv[4], 0, 0));
-			else
-				r=(*tab[i].func)(arg[0], arg[1], arg[2], arg[3], arg[4]);
-			if(r == -1){
-				errstr(ebuf, sizeof ebuf);
-				fprint(2, "syscall: return %lld, error:%s\n", r, ebuf);
-			}else{
-				ebuf[0] = 0;
-				fprint(2, "syscall: return %lld, no error\n", r);
-			}
-			if(oflag)
-				print("%s\n", buf);
-			if(xflag){
-				for(j=0; j<r; j++){
-					if(j%16 == 0)
-						print("%.4x\t", j);
-					c = buf[j]&0xFF;
-					if('!'<=c && c<='~')
-						print(" %c ", c);
-					else
-						print("%.2ux ", c);
-					if(j%16 == 15)
-						print("\n");
-				}
-				print("\n");
-			}
-			if(sflag && r > 0){
-				r = convM2D((uchar*)buf, r, &d, strs);
-				if(r <= BIT16SZ)
-					print("short stat message\n");
-				else{
-					print("[%s] ", d.muid);
-					print("(%.16llux %lud %.2ux) ", d.qid.path, d.qid.vers, d.qid.type);
-					print("%M (%luo) ", d.mode, d.mode);
-					print("%c %d ", d.type, d.dev);
-					print("%s %s ", d.uid, d.gid);
-					print("%lld ", d.length);
-					print("%s ", lstime(d.mtime));
-					print("%s\n", d.name);
-					print("\tmtime: %s\n\tatime: %s\n", xctime(d.mtime), xctime(d.atime));
-				}
-			}
-			exits(ebuf);
+	if(r == -1){
+		errstr(ebuf, sizeof ebuf);
+		fprint(2, "syscall: return: %lld error: %s\n", r, ebuf);
+		exits(ebuf);
+	}
+	fprint(2, "syscall: return: %lld\n", r);
+	if(oflag){
+		nbuf = r;
+		switch(i){
+		case _ERRSTR: case ERRSTR: case FD2PATH:
+			nbuf = strlen(buf);
 		}
-	fprint(2, "syscall: %s not known\n", argv[0]);
-	exits("unknown");
+		if(write(1, buf, nbuf) != nbuf)
+			sysfatal("write: %r");
+	}else if(sflag){
+		r = convM2D((uchar*)buf, r, &d, strs);
+		if(r <= BIT16SZ)
+			print("short stat message\n");
+		else
+			print("%D\n", &d);
+	}
+	exits(nil);
 }
-
-uintptr
-parse(char *s)
-{
-	char *t;
-	uintptr l;
-
-	if(strcmp(s, "buf") == 0)
-		return (uintptr)buf;
-	
-	l = strtoull(s, &t, 0);
-	if(t>s && *t==0)
-		return l;
-	return (uintptr)s; 
-}
-
-void
-catch(void *, char *msg)
-{
-	fprint(2, "syscall: received note='%s'\n", msg);
-	noted(NDFLT);
-}

Re: [9front] [PATCH] syscall: utility overhaul

[Ethan Gardener]

On Sat, Sep 19, 2020, at 2:05 PM, kvik@a-b.xyz wrote:
> 
> * The -o flag outputs the entire buffer to the length returned
>   by the syscall, or, in case of fd2path(2) and errstr(2), to '\0'.

If a user redirects this output to a file and tries to edit it, sam and acme will remove the \0 bytes and issue a warning on load. Ed doesn't warn, but truncates lines at \0. Just raising the issue in case it's relevant. IMO, these are minor bugs in our editors.

> * The -x flag is removed; the above makes it possible to pipe
>   into xd(1) to get the same result.

I'm not familiar with syscall, but is it really the same result?

Re: [9front] [PATCH] syscall: utility overhaul

[kvik]

> If a user redirects this output to a file and tries to edit it,
> sam and acme will remove the \0 bytes and issue a warning on load.
> Ed doesn't warn, but truncates lines at \0. Just raising the issue in
> case it's relevant. IMO, these are minor bugs in our editors.

This is not something that syscall(1) should worry about.  If you asked
to do a syscall, then you must expect to get the result of a syscall.

> > * The -x flag is removed; the above makes it possible to pipe
> >   into xd(1) to get the same result.
> 
> I'm not familiar with syscall, but is it really the same result?

It's not, at least not by default.  However, you can invoke xd(1) in just
the right way to get the /same/ result, if you so please.