Re: [9front] ramfs usage

[Jacob Moody <moody@posixcafe.org>]

My vote would be to keep things as is.

While it is not a great precedent, a large chunk of code
from what I've seen don't throw errors for nonsensical combinations.
If doing things from scratch I would think perhaps its worth the effort
to check for, but it is probably too late to set this precedent and modifying
programs after the fact. I just don't want this to be the start of this happening
to every program.

A lot of these man page changes seem like a general rewrite, in particular the
change of "option" to "flag" which feels off.

It just doesn't seem the benefits outweigh the cost of the churn to me.


Thanks,
Moody

On 6/19/24 15:36, ori@eigenstate.org wrote:
> Hm. I'll talke a look at this; I wonder if it's too late
> to clean up the interaction of the flags. Probably :(
> 
> Quoth me+unobe@fallglow.com:
>> I was looking at ramfs and noticed that the ordering of some flags
>> provided inconsistent behavior.  For example:
>>
>>     cpu% ramfs -s -m /n/foo
>>     cpu% cpu% ls -l /n
>>     d-rwxrwxrwx M 83820 david david 0 May 16 12:29 /n/foo
>>     cpu% ramfs -m /n/boo -s
>>     cpu% ls -l /n
>>     d-rwxrwxrwx M 83788 david david 0 May 16 12:29 /n/foo
>> 	cpu%
>>
>> To address this, I think that -m should always add a mountpoint,
>> unless -i is used.
>>
>> I also noticed that:
>>
>>  * a few of the (non-debugging) flags were not documented in the man
>>  page: -b, -a, and -c to set the mount flags.
>>
>>  * -i is mutually exclusive with -s, -S, and -m, -b, -a, and -c but
>>  that isn't clearly spelled out.  If -s, -S, and -m are used together,
>>  that seems like a fatal error to me, but if -b, -a, -c are used, that
>>  is an ignorable error that can be mentioned on stderr. For instance:
>>
>>     cpu% ramfs -i -b
>>     mount flags ignored when using -i: 
>> 	cpu% ramfs -s -i
>>     ramfs: -i cannot be used with -s, -S, or -m flag
>> 	cpu% 
>>
>>  * A sysfatal() error would never happen because mtpt is always
>>  defined when srvname is not, and vice versa, because mtpt is set by
>>  default and only unset when srvname is set.
>>
>> I offer the attached patch for consideration.
>>
>> From: Romano <me+git@fallglow.com>
>> Date: Thu, 16 May 2024 21:52:36 +0000
>> Subject: [PATCH] ramfs: document usage and provide clearer usage errors
>>
>>
>> Allow -m to be used to set mountpoint when used with -s and -S,
>> of ordering. Document flags -b, -a, and -c; document interaction
>> between certain flags, and raise errors when mis-use occurs.
>> ---
>> diff 7089a0e8dc50d86dce93176d284e23f82e5d2bdf 0b55b43b99d97129a0ca0731243d1edc7bbbb6ae
>> --- a/sys/man/4/ramfs
>> +++ b/sys/man/4/ramfs
>> @@ -4,7 +4,7 @@
>>  .SH SYNOPSIS
>>  .B ramfs
>>  [
>> -.B -Dipsu
>> +.B -Dipsubac
>>  ]
>>  [
>>  .B -m
>> @@ -15,30 +15,50 @@
>>  .I srvname
>>  ]
>>  .SH DESCRIPTION
>> +.PP
>>  .I Ramfs
>> -starts a process that mounts itself (see
>> -.IR bind (2))
>> -on
>> -.I mountpoint
>> -(default
>> -.BR /tmp ).
>> +implements a file tree which keeps all files in memory.
>> +Initially the file tree is empty.
>> +.PP
>>  The
>>  .I ramfs
>> -process implements a file tree rooted at
>> -.IR dir ,
>> -keeping all files in memory.
>> -Initially the file tree is empty.
>> +process mounts the
>> +.IR mountpoint (or
>> +.B /tmp ,
>> +if not provided)
>> +with the mount flags
>> +.B MREPL and
>> +.B MCREATE .
>> +The mount flags can be set explicitly
>> +using
>> +.B -b ,
>> +.B -a , and
>> +.B -c ,
>> +corresponding respectively to the flags
>> +.B MBEFORE ,
>> +.B MAFTER , or
>> +.B MCREATE (see
>> +.IR bind(2)).
>>  .PP
>>  The
>> -.B -D
>> -option enables a trace of general debugging messages.
>> +.B -p
>> +flag causes
>> +.I ramfs
>> +to make its memory `private'
>> +(see
>> +.IR proc (3))
>> +so that its files are not accessible through the debugging interface.
>>  .PP
>>  The
>> +.B -D
>> +flag enables a trace of general debugging messages.
>> +.PP
>>  .B -i
>>  flag tells
>>  .I ramfs
>>  to use file descriptors 0 and 1 for its communication channel
>> -rather than create a pipe.
>> +rather than create a pipe or mount at
>> +.IR mountpoint .
>>  This makes it possible to use
>>  .I ramfs
>>  as a file server on a remote machine: the file descriptors 0
>> @@ -47,15 +67,6 @@
>>  to the client machine.
>>  .PP
>>  The
>> -.B -p
>> -flag causes
>> -.I ramfs
>> -to make its memory `private'
>> -(see
>> -.IR proc (3))
>> -so that its files are not accessible through the debugging interface.
>> -.PP
>> -The
>>  .B -s
>>  .RB ( -S )
>>  flag causes
>> @@ -62,17 +73,26 @@
>>  .I ramfs
>>  to post its channel on
>>  .B /srv/ramfs
>> -.RB ( /srv/ \fIsrvname\fR)
>> -rather than mounting it on
>> -.IR mountpoint ,
>> +.RB ( /srv/ \fIsrvname\fR) ,
>>  enabling multiple clients to access its files.
>> -However, it does not authenticate its clients and its
>> +The flag also causes
>> +.I ramfs
>> +to not use the default
>> +.IR mountpoint
>> +.B /tmp ,
>> +however it will use the mountpoint
>> +.IR mountpoint
>> +explicit;y set using the
>> +.B -m
>> +flag.
>> +.I Ramfs
>> +does not authenticate its clients and its
>>  implementation of groups is simplistic, so
>>  it should not be used for precious data.
>>  .PP
>>  The
>>  .B -u
>> -option permits
>> +flag permits
>>  .I ramfs
>>  to consume as much memory as needed;
>>  without it,
>> @@ -86,4 +106,5 @@
>>  .SH SOURCE
>>  .B /sys/src/cmd/ramfs.c
>>  .SH "SEE ALSO"
>> -.IR bind (2)
>> +.IR bind (2) ,
>> +.IR proc (3)
>> --- a/sys/src/cmd/ramfs.c
>> +++ b/sys/src/cmd/ramfs.c
>> @@ -455,27 +455,40 @@
>>  {
>>  	char *srvname = nil;
>>  	char *mtpt = "/tmp";
>> -	int mountflags, stdio;
>> +	int marg, mountflags, stdio;
>>  
>>  	fs.tree = alloctree(nil, nil, DMDIR|0777, fsdestroyfile);
>>  
>> -	mountflags = stdio = 0;
>> +	marg = mountflags = stdio = 0;
>>  	ARGBEGIN{
>>  	case 'D':
>>  		chatty9p++;
>>  		break;
>>  	case 's':
>> +		if(stdio)
>> +			sysfatal("-s cannot be used with -i flag");
>>  		srvname = "ramfs";
>> -		mtpt = nil;
>> +		if(marg == 0)
>> +			mtpt = nil;
>>  		break;
>>  	case 'S':
>> +		if(stdio)
>> +			sysfatal("-S cannot be used with -i flag");
>> +		if(srvname)
>> +			sysfatal("-s cannot be used with -S");
>>  		srvname = EARGF(usage());
>> -		mtpt = nil;
>> +		if(marg == 0)
>> +			mtpt = nil;
>>  		break;
>>  	case 'm':
>> +		if(stdio)
>> +			sysfatal("-m cannot be used with -i flag");
>>  		mtpt = EARGF(usage());
>> +		marg = 1;
>>  		break;
>>  	case 'i':
>> +		if(srvname || marg)
>> +			sysfatal("-i cannot be used with -s, -S, or -m flag");
>>  		stdio = 1;
>>  		break;
>>  	case 'p':
>> @@ -500,14 +513,14 @@
>>  		usage();
>>  
>>  	if(stdio){
>> +		if(mountflags)
>> +			perror("mount flags ignored when using -i");
>> +
>>  		fs.infd = 0;
>>  		fs.outfd = 1;
>>  		srv(&fs);
>>  		exits(0);
>>  	}
>> -
>> -	if(srvname == nil && mtpt == nil)
>> -		sysfatal("must specify -S, or -m option");
>>  
>>  	if(mountflags == 0)
>>  		mountflags = MREPL | MCREATE;
>