Subject: Re: Adding to KNF.
To: Darren Reed <darrenr@reed.wattle.id.au>
From: Julio Merino <jmmv@hispabsd.org>
List: tech-kern
Date: 06/15/2002 10:22:37
On Sat, 15 Jun 2002 15:23:11 +1000 (EST)
"Darren Reed" <darrenr@reed.wattle.id.au> 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.
Yes... the few things I've done for the kernel were a bit complex
because the lack of comments or documentation :p
> - statement about what the function does, including what is considered
> to be a successful call or not. This may or may not include talking
> about what gets returned.
Very important...
> To go through and do this retrospectively is a huge task so I am not
> suggesting that, rather as people touch various routines where they
> may or may not alter behaviour, more comments get added. I think that
> updating the KNF is the correct way to encourage more people to do
> this. A suggestion for pre-function comments might look like is this:
>
> /*
> * Function: foo
> * Locks: big_lock(M)
> */
>
> Where "M" is a mutex. Some set of mneumonics needs to be standardised
> upon here to make the lock list both meaningful and concise.
>
> /*
> * Function: foo_rx
> * Interrupts: imp
> */
>
> "imp" from "splimp".
Look ok, but they miss the description.
> An example from IPFilter:
> /* ------------------------------------------------------------------------ */
> /* Function: nat_new */
> /* Returns: nat_t* - NULL == failure to create new NAT structure, */
> /* else pointer to new NAT structure */
> /* Parameters: fin(I) - pointer to packet information */
> /* np(I) - pointer to NAT rule */
> /* natsave(I) - pointer to where to store NAT struct pointer */
> /* flags(I) - flags describing the current packet */
> /* direction(I) - direction of packet (in/out) */
> /* Write Lock: ipf_nat */
> /* */
> /* Attempts to create a new NAT entry. Does not actually change the packet */
> /* in any way. */
> /* */
> /* This fucntion is in three main parts: (1) deal with creating a new NAT */
> /* structure for a "MAP" rule (outgoing NAT translation); (2) deal with */
> /* creating a new NAT structure for a "RDR" rule (incoming NAT translation) */
> /* and (3) building that structure and putting it into the NAT table(s). */
> /* ------------------------------------------------------------------------ */
>
> Extremely verbose, and perhaps over verbose on the "Parameters" side.
> I'm not saying NetBSD should use that but perhaps think into the future,
> now, about what would be good to have and set a direction for others
> here to follow when hacking on NetBSD code.
Quite big but useful ;) I tend to dislike seeing "drawing" in comments,
like the ------- at the start and the end, the */ at the end of each
line... but well, that's IMHO and I do not decide. lol.
Though, I say again, I agree with your idea. Lets see if more people
gets interested.
Regards
--
HispaBSD admin - http://www.hispabsd.org
Julio Merino <jmmv@hispabsd.org>