Subject: Re: Adding to KNF.
To: Darren Reed <darrenr@reed.wattle.id.au>
From: Bill Studenmund <wrstuden@netbsd.org>
List: tech-kern
Date: 06/15/2002 15:56:58
On Sat, 15 Jun 2002, Darren Reed wrote:

> In some email I received from David Laight, sie wrote:
> > >
> > > One of my pet hates is the lack of documentation with much of the code,
> > > ie. comments.  As we move forward, more and more code gets written and
> > > it doesn't get any easier to understand how it works.  At present we
> > > are without much in the way of SMP support but once that is added, the
> > > code will become even more complex, over time.
> > >
> > > What I'd like to suggest is that in going forward, we adopt some changes
> > > to the KNF that require people to at least put more text in a comment
> > > block prior to the function.

I think this is a good idea. Not that we will get 100% compliance on new
code, but adding it to KNF would 1) point folks in the right direction
(towards actually documenting), and 2) will give a guide as to what should
be in the documentation.

> > I hate mandatory comment blocks before functions!

Well, that's your opinion. :-)

I personally like them.

> > They tend to end up telling you stuff that can be figured out
> > immediately - rather than stuff that is slightly obscure.
>
> There is nothing wrong with pointing out the obvious.  At least it
> makes it explicitly clear that the obvious interpretation is correct.

It also depends. I don't think we're suggesting a screen full of
documentation on a simple 5 line function. And I think der Mouse put it
best:

> I've written one-line comments for whole-screen functions, and
> conversely I've written multiple screenfuls of comment for ten-line
> functions.  It's all a judgement call....

And I think it would be a good thing to nudge folks to writing something.

> How do you audit a function for correct functionality if there is no
> statement about what it is meant to do ?

Good point.

> This is actually the crux of what I'm on about here.  I'm seeing too
> much "new" code written that has little or no explanation about what
> it is meant to be doing to feel comfortable about it doing the right
> thing.  There is no way you can convince me that a function is "correct"
> as a result of an "audit" if there is no comment block stating what it
> does (or is meant to do).

I don't think "new" code alone suffers from this problem. :-)

> > > Some things which I think would be or are going to be useful:
> > >
> > > - statement about which locks are expected to be held and how.
> > >   this should include extra comments if a lock is acquired or released
> > >   prior to returning.
> >
> > It is MUCH more important to add comments to structure definitions
> > indicating what the fields are for, and which lock covers each
> > field.  Then you can CHECK whether the routine is doing the right thing.
>
> That's good for locks that protect individual fields within a structure,
> but what about locks for tables, etc ?

I don't think having comments above code blocks will (or should) preclude
putting comments above structures.

But having info about, "accessing this field must be done with this lock
held," doesn't help if you have one routine calling another that access
such a field w/o locking. Is it the second routine's fault for not
locking, or was the first routine supposed to lock and then call the
second routine?

> > Comment blocks (esp. those above functions) get out of date very
> > easily.
>
> Well, that is another problem altogether.  People updating code should
> also be updating comments, as appropriate.  Software development is not
> just about cutting code.  Bugs introduced in comments are just as henious
> as bugs introduced in code.

Indeed.

On Sun, 16 Jun 2002, Darren Reed wrote:

> In some email I received from xs@kittenz.org, sie wrote:
> > Isn't a section 9 manual page a good place to put this kind of thing?
> > Under .Sh LOCKS or some such.
> > Obviously not every function needs a man page, but since there are so
> > many functions and structures documented in this way already, it means
> > there's less work. It's also less of an eye sore for those that do not
> > wish to see it.
>
> No, man pages are not the right place to put this kind of information.
>
> This needs to be *with* the source code so that when you're working on
> it the information is *there*, not somewhere else.

Well, I think there's some of the documentation that should be in man
pages and some that should be in the code. A broad description of (for
example) locks would do best in a man page, but I think the code is a
better place to document which of these or those routines expects this
lock to be held when it is called.

Take care,

Bill