Hi Theo,

Theo de Raadt wrote on Fri, Oct 30, 2020 at 09:59:09AM -0600:

> With a careful reading of the current manual page, everything is there
> and it is accurate.
> 
>     With an argument naming an existing file
>                                ^^^^^^^^
> 
> Ok so let's create it with touch.  Probably has wrong permissions.
> But now accton to that file works.  Or enable it and reboot, and now
> disable it and reboot, and the file still exists, so now accton works
> because it is an existing file (with the right permissions I guess).
> 
> So it *IS* working as documented.  It is just a bit weird, because the
> accton command (and system call) do not create the file.
> 
> 
> My problem with these changes is this is the man page for the accton(8)
> command, it documents *the binary*.  The manpage has already been subverted
> talk about rcctl, and about how /etc/rc runs the command.  But the man
> page should first and foremost be about the command, not about /etc/rc
> and rcctl, am i not right?

You are right.

The paragraph about rc.conf.local(8) is short (one sentence, one
line and three half lines) and near the end of the DESCRIPTION,
which is appropriate IMHO.  It feels more prominent than it should
only because the accton(8) command itself is so simple and
straightforward that it can be described exhaustively in three
lines of text.

> For instance, the ntpd manual page has a tiny section about rc.conf.

That paragraph has more or less the same length as the one in accton(8).

> So in conclusion, I think both of you are writing text about the startup
> subsystem, into the wrong manual page.  I think both proposals are skewed.
>
> So questions.
>
> 1 - historically it requires a file to be pre-created.  In the rc scripts,
>     this is a touch.  That grabs the umask and ownership of root's run of
>     /etc/rc.
> 2 - could we do better, in some way?
> 3 - accton system call does not have a umask, is that where this design came
>     from?
> 4 - could we improve upon this?
> 5 - could accton always (attempt to) create the file, without harming
>     existing use cases, with proper owner and chmod flags?

Maybe.  It would change behaviour, though.

Somebody might rely on the fact that accton(8) is a NOOP when the
file given as an argument does not exist.  I'm not saying that is
necessarily a good idea, but someone might have done it, for
example because they have to maintain a herd of similar machines,
want accounting on some but not on others, deploy the same
/etc/rc.conf.local to all and create or delete the file as desired.

Having the kernel or accton(8) create the file if it does not exist
would make the tool slightly easier to use for most users.  Is that
worth asking users doing soemthing like the above to change their
deployment?  Maybe, but i'm not sure.  The advantages feel small
either way.

> 6 - or should that be tied to a flag, making it easier to document?

I somewhat dislike that.  Either we regard creating the file as
part of the job.  Then accton(8) ought to do it without a flag, and
people who don't want accounting should not call it with an argument
in the first place.  Or we regard creating and deleting the file
as a separate decision, like we do it now.  Then accton(8) should
not meddle with it, not even as a matter of a flag.  Besides,
creating a root:wheel 0644 file is not rocket science requiring an
additional option in some specialized program to be accomplished.
For that purpose, several adequate and flexible tools already exist,
including install(1).


Triggered by looking at this manual page: Let's trim the ridiculous
HISTORY section.  No, it didn't exist forever.  No, the manual page
isn't new.

A predecessor "gacct(8)" may have existed briefly in v4, but i'm
not wholly convinced.  There is only a skeleton manual page

  https://minnie.tuhs.org/cgi-bin/utree.pl?file=V4/man/manx/gacct.8

not even containing a DESCRIPTION.  I found no code, and it seems
it probably wasn't contained in v5 nor in v6.  So it seems best
to not mention it.

OK?
  Ingo


Index: accton.8
===================================================================
RCS file: /cvs/src/usr.sbin/accton/accton.8,v
retrieving revision 1.11
diff -u -p -r1.11 accton.8
--- accton.8    10 Nov 2019 20:51:53 -0000      1.11
+++ accton.8    30 Oct 2020 16:43:55 -0000
@@ -73,4 +73,5 @@ default accounting file
 .Sh HISTORY
 The
 .Nm
-command has existed nearly forever, but this man page is new.
+command first appeared in
+.At v7 .

Reply via email to