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>