Date:   Wed, 19 Feb 2020 16:25:27 +0100
From:   Stephen Kitt <steve@....org>
To:     Jonathan Corbet <corbet@....net>
Cc:     Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 7/8] docs: add a script to check sysctl docs

Hi Jon,

On Wed, 19 Feb 2020 03:44:16 -0700, Jonathan Corbet <corbet@....net> wrote:
> On Tue, 18 Feb 2020 13:59:22 +0100
> Stephen Kitt <steve@....org> wrote:
> 
> > This script allows sysctl documentation to be checked against the
> > kernel source code, to identify missing or obsolete entries. Running
> > it against 5.5 shows for example that sysctl/kernel.rst has two
> > obsolete entries and is missing 52 entries.
> > 
> > Signed-off-by: Stephen Kitt <steve@....org>  
> 
> So I like the idea here, but I have a couple of nits and one larger
> thought...
> > ---
> >  Documentation/admin-guide/sysctl/kernel.rst |   3 +
> >  scripts/check-sysctl-docs                   | 181 ++++++++++++++++++++
> >  2 files changed, 184 insertions(+)
> >  create mode 100755 scripts/check-sysctl-docs
> > 
> > diff --git a/Documentation/admin-guide/sysctl/kernel.rst
> > b/Documentation/admin-guide/sysctl/kernel.rst index
> > a67c218c4066..3d21e076aea4 100644 ---
> > a/Documentation/admin-guide/sysctl/kernel.rst +++
> > b/Documentation/admin-guide/sysctl/kernel.rst @@ -2,6 +2,9 @@
> >  Documentation for /proc/sys/kernel/
> >  ===================================
> >  
> > +.. See scripts/check-sysctl-docs to keep this up to date
> > +
> > +
> >  Copyright (c) 1998, 1999,  Rik van Riel <riel@...linux.org>
> >  
> >  Copyright (c) 2009,        Shen Feng<shen@...fujitsu.com>
> > diff --git a/scripts/check-sysctl-docs b/scripts/check-sysctl-docs
> > new file mode 100755
> > index 000000000000..b3b47c188a2d
> > --- /dev/null
> > +++ b/scripts/check-sysctl-docs
> > @@ -0,0 +1,181 @@
> > +#!/usr/bin/gawk -f
> > +# SPDX-License-Identifier: GPL-2.0-only  
> 
> By Documentation/process/license-rules.rst, that should be "GPL-2.0"; the
> absence of a "+" means "only".

Ah, thanks, I hadn’t noticed that. I’m used to the SPDX identifiers listed on
the LF website, https://spdx.org/licenses/GPL-2.0-only.html in this case.
(I’m not alone apparently, see “git grep GPL-2.0-only”...)

> > +# Script to check sysctl documentation against source files
> > +#
> > +# Copyright © 2020 Stephen Kitt  
> 
> Some people object to the introduction of unnecessary non-ASCII text, and
> this one would count; can we take the © symbol out?

Yes, of course!

> Now for the bigger thought...  I wonder if what we really want to do is
> to adopt some form of the kerneldoc format for sysctl knobs?  That would
> allow the documentation to be placed in the source right next to the
> table entries, which might, in the optimistic world I inhabit, help
> developers to keep them up to date.
> 
> That, of course, is more work than what you've done; until somebody feels
> inspired to do that work I'll happily accept a tweaked version of this
> script.  But one can always dream...

Right, that occurred to me too. In fact, it seems to me that in an ideal
world, we wouldn’t even have the big tables of sysctl entries, since that’s
another level of indirection — in most cases, the relevant information is
available where the sysctl variables are declared, not where they’re
registered in the tables. This leads to disconnects between the reality of
the code and sysctl table entries, for example ieee_emulation_warnings which
is (for now, I’ve submitted a patch which has been accepted) still present in
sysctl.c but isn’t used.

I’ll fix up the script and re-submit it.

Regards,

Stephen

Content of type "application/pgp-signature" skipped

Powered by blists - more mailing lists