Re: rarpd: clarify synopsis

2022-10-02 Thread Theo de Raadt
Klemens Nanni  wrote:

> rarpd(8) is small enough where my impression is that refining it a
> little would be good, but it quickly comes down to personal taste.

And I continue to disagree.

Another example of the same pattern is ifconfig.  Here you will see it
is not documented that [-a] is incompatible with [interface], but the
program does actually enforce it (refuses to act).

Even later in the man page, this is not actually described.  Yet noone is
dying from this.

The synopsis / usage language simply cannot describe this, and I argue
that it SHOULD NOT describe the situation, because past attempts at
being complete precise have made the SYNOPSIS / usage cluttered, and
thus less valuable to the users.

Another way of looking at it is that the grammer is permissive, rather
than proscriptive.

So I suggest you get over it.



Re: rarpd: clarify synopsis

2022-10-02 Thread Klemens Nanni
On Sun, Oct 02, 2022 at 02:52:37PM +0100, Jason McIntyre wrote:
> On Sun, Oct 02, 2022 at 01:07:04PM +, Klemens Nanni wrote:
> > rarpd(8) either "Listen[s] on all the Ethernets attached to the system"
> > or requires an explicit list, not both:
> > 
> > $ rarpd -a em0
> > usage: rarpd [-adflt] if0 [... ifN]
> > $ ./obj/rarpd -a em0
> > usage: rarpd [-dflt] -a | if ...
> > 
> > Or would this be better?
> > rarpd [-dflt] if ...
> > rarpd -a [-dflt]
> > 
> > Feedback? OK?
> > 
> 
> please don't!

Let's just leave it as is, then.

> 
> - if you are specifying -a, you would hardly expect to give a list of
>   interfaces too. the manual documents it clearly. why single out -a?

Yes, the manual makes that clear, but the usage/synopsis does not.
I personally prefer such things to be visible right away, but that might
just be me.

> - with the newer version, it is unclear whether you can specify any
>   flags at all with "if ...". we could tighten that, but it's still
>   messy.

You mean it could mean either '[-dflt] -a' or 'if ...'?
Fair point, that's not better.

> - and splitting such a simple SYNOPSIS in two would be nuts.

Not a big deal to me;  cut(1) seems a bit similar in that it is clearly
split into three rather than an ambiguous one.

> 
> - if anything, i think we could trim "if0 [... ifN]" to just "if ...".
>   so i agree with that part.

That's a general simplification we could do in a few more manuals.



Re: rarpd: clarify synopsis

2022-10-02 Thread Klemens Nanni
On Sun, Oct 02, 2022 at 10:09:32AM -0600, Theo de Raadt wrote:
> The getopt language is imprecise, and attempts to be precise with it
> usually go poorly.
> 
> For example,
> 
> SYNOPSIS
>  ls [-1AaCcdFfgHhikLlmnopqRrSsTtux] [file ...]
> 
> % ls -1AaCcdFfgHhikLlmnopqRrSsTtux
> 
> The result may seem surprising.  I claim the result is not surprising.
> 
> It is unsurprising because the getopt langauge makes no claim about
> which options are prioritized over other options, nor does it make
> claims about which option combinations will be rejected and result
> in error.
> 
> It only lists possibilities.  Sometimes the words in the manual page
> describe the subset that will work, but sometimes it doesn't.  Sometimes
> a program enforces a strict set of combinations, and somethings there
> are surprises.

You point about getopt not being that precise (by design) makes sense.
Many manuals differ in this regard and yet I still consider each of them
fine as they are.

> And that's fine.  Imagine trying to change the ls SYNOPSIS into 15 valid
> combination so options, and then enforcing the priority and overlap
> inside ls.c, and describing the reason why the priorites are chosen as
> such (and probably introducing incompatibilities with other systems in
> the process).  I think that would be a gigantic waste of time to do just
> in ls, and I think we should only try to improve this in extremely
> narrow cases when it really matters.

I agree, ls(1) seems like the ultimate case where splitting things up is
a bad idea.

rarpd(8) is small enough where my impression is that refining it a
little would be good, but it quickly comes down to personal taste.



Re: rarpd: clarify synopsis

2022-10-02 Thread Theo de Raadt
The getopt language is imprecise, and attempts to be precise with it
usually go poorly.

For example,

SYNOPSIS
 ls [-1AaCcdFfgHhikLlmnopqRrSsTtux] [file ...]

% ls -1AaCcdFfgHhikLlmnopqRrSsTtux

The result may seem surprising.  I claim the result is not surprising.

It is unsurprising because the getopt langauge makes no claim about
which options are prioritized over other options, nor does it make
claims about which option combinations will be rejected and result
in error.

It only lists possibilities.  Sometimes the words in the manual page
describe the subset that will work, but sometimes it doesn't.  Sometimes
a program enforces a strict set of combinations, and somethings there
are surprises.

And that's fine.  Imagine trying to change the ls SYNOPSIS into 15 valid
combination so options, and then enforcing the priority and overlap
inside ls.c, and describing the reason why the priorites are chosen as
such (and probably introducing incompatibilities with other systems in
the process).  I think that would be a gigantic waste of time to do just
in ls, and I think we should only try to improve this in extremely
narrow cases when it really matters.

Klemens Nanni  wrote:

> rarpd(8) either "Listen[s] on all the Ethernets attached to the system"
> or requires an explicit list, not both:
> 
>   $ rarpd -a em0
>   usage: rarpd [-adflt] if0 [... ifN]
>   $ ./obj/rarpd -a em0
>   usage: rarpd [-dflt] -a | if ...
> 
> Or would this be better?
>   rarpd [-dflt] if ...
>   rarpd -a [-dflt]
> 
> Feedback? OK?
> 
> Index: rarpd.8
> ===
> RCS file: /cvs/src/usr.sbin/rarpd/rarpd.8,v
> retrieving revision 1.21
> diff -u -p -r1.21 rarpd.8
> --- rarpd.8   28 Oct 2015 10:02:59 -  1.21
> +++ rarpd.8   29 Sep 2022 21:04:29 -
> @@ -29,8 +29,8 @@
>  .Nd reverse ARP daemon
>  .Sh SYNOPSIS
>  .Nm rarpd
> -.Op Fl adflt
> -.Ar if0 Op Ar ... ifN
> +.Op Fl dflt
> +.Fl a | Ar if ...
>  .Sh DESCRIPTION
>  .Nm
>  services Reverse ARP requests on the Ethernet connected to
> Index: rarpd.c
> ===
> RCS file: /cvs/src/usr.sbin/rarpd/rarpd.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 rarpd.c
> --- rarpd.c   15 Nov 2021 15:14:24 -  1.79
> +++ rarpd.c   29 Sep 2022 21:05:35 -
> @@ -217,7 +217,7 @@ init_all(void)
>  __dead void
>  usage(void)
>  {
> - (void) fprintf(stderr, "usage: rarpd [-adflt] if0 [... ifN]\n");
> + fprintf(stderr, "usage: rarpd [-dflt] -a | if ...\n");
>   exit(1);
>  }
>  
> 



Re: rarpd: clarify synopsis

2022-10-02 Thread Jason McIntyre
On Sun, Oct 02, 2022 at 01:07:04PM +, Klemens Nanni wrote:
> rarpd(8) either "Listen[s] on all the Ethernets attached to the system"
> or requires an explicit list, not both:
> 
>   $ rarpd -a em0
>   usage: rarpd [-adflt] if0 [... ifN]
>   $ ./obj/rarpd -a em0
>   usage: rarpd [-dflt] -a | if ...
> 
> Or would this be better?
>   rarpd [-dflt] if ...
>   rarpd -a [-dflt]
> 
> Feedback? OK?
> 

please don't!

- if you are specifying -a, you would hardly expect to give a list of
  interfaces too. the manual documents it clearly. why single out -a?

- with the newer version, it is unclear whether you can specify any
  flags at all with "if ...". we could tighten that, but it's still
  messy.

- and splitting such a simple SYNOPSIS in two would be nuts.

- if anything, i think we could trim "if0 [... ifN]" to just "if ...".
  so i agree with that part.

jmc

> Index: rarpd.8
> ===
> RCS file: /cvs/src/usr.sbin/rarpd/rarpd.8,v
> retrieving revision 1.21
> diff -u -p -r1.21 rarpd.8
> --- rarpd.8   28 Oct 2015 10:02:59 -  1.21
> +++ rarpd.8   29 Sep 2022 21:04:29 -
> @@ -29,8 +29,8 @@
>  .Nd reverse ARP daemon
>  .Sh SYNOPSIS
>  .Nm rarpd
> -.Op Fl adflt
> -.Ar if0 Op Ar ... ifN
> +.Op Fl dflt
> +.Fl a | Ar if ...
>  .Sh DESCRIPTION
>  .Nm
>  services Reverse ARP requests on the Ethernet connected to
> Index: rarpd.c
> ===
> RCS file: /cvs/src/usr.sbin/rarpd/rarpd.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 rarpd.c
> --- rarpd.c   15 Nov 2021 15:14:24 -  1.79
> +++ rarpd.c   29 Sep 2022 21:05:35 -
> @@ -217,7 +217,7 @@ init_all(void)
>  __dead void
>  usage(void)
>  {
> - (void) fprintf(stderr, "usage: rarpd [-adflt] if0 [... ifN]\n");
> + fprintf(stderr, "usage: rarpd [-dflt] -a | if ...\n");
>   exit(1);
>  }
>  
>