On Fri, Oct 30, 2020 at 09:59:09AM -0600, Theo de Raadt wrote:
> Jason McIntyre <j...@kerhand.co.uk> wrote:
> 
> > On Fri, Oct 30, 2020 at 04:24:43PM +0100, Solene Rapenne wrote:
> > > reading accton(8) it's not clear that if you
> > > enable it you need to restart the system for
> > > accounting to be effective.
> > > 
> > > Here is a change to add the explanation, but
> > > I'm not sure if the wording is correct.
> > > 
> > 
> > hi. i think the text that follows is really trying to say the same thing
> > (you enable it at boot, so until you boot it isn;t enabled). we could
> > just try to make that one bit of text a bit clearer:
> > 
> >     .Nm
> >     should be enabled at boot time, either by running
> >     .Dq rcctl enable accounting
> >     or by setting
> >     .Dq accounting=YES
> >     in
> >     .Xr rc.conf.local 8 .
> 
> With a careful reading of the current manual page, everything is there
> and it is accurate.
> 
>     With an argument naming an existing file
>                                ^^^^^^^^
> 

correct.  i missed that accton could be started this way, and
concentrated on how to start it at boot (which i guess is nromally the
way it'll be run).

> 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.
> 

yes, it works as advertised. and all the bits are there - i just thought
solene's notes were a repetition on the current boot notes.

> 
> 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?  For instance, the ntpd manual page has a tiny
> section about rc.conf.
> 

the main text body is so slim that the boot notes look bigger in
relation. ntpd has more to say.

but rerading ntpd(8), i think it discusses the startup process in a
better way. maybe we should move to a text more similar to that one/

i think it's still fair game to say how to start a process in its man
page. if we don;t want such information, we have a lot of undoing to er,
do.

> 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?
> 6 - or should that be tied to a flag, making it easier to document?
> 

i can;t answer these points. except with more questions:

- is it fair game for a process to document how it fits into the startup
  process? right now, we broadly do that. i think it's fine.

- somewhere we switched to telling people how to use rcctl. i never like
  this system, but it was broadly agreed. so now there are two ways to
  do things. should the pages describe both? it's hard to see we'd do
  one without the other. i note that ntpd(8) does not discuss
  rcctl(8)...

jmc

> > sth like that?
> > jmc
> > 
> > > Index: accton.8
> > > ===================================================================
> > > RCS file: /home/reposync/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 15:22:14 -0000
> > > @@ -43,6 +43,10 @@ causes system accounting information for
> > >  to be placed at the end of the file.
> > >  If no argument is given, accounting is turned off.
> > >  .Pp
> > > +Accounting is done if it was enabled when system booted.
> > > +If you activate accounting, a reboot is required for it to become
> > > +effective.
> > > +.Pp
> > >  To have
> > >  .Nm
> > >  enabled at boot time, use
> > > 
> > 
> 

Reply via email to