Re: [Syllable-kernel] C99 kernel source preview - needs more work

[Daniel G. <da...@fp...>]

Take two, now that the list works again.

On Tue, 2005-02-15 at 23:38 -0800, Jake Hamby wrote:
>I'm about 90% completed with my C99 typesafe conversion of the kernel 
>sources, and the code now compiles with few warnings (and the
remaining 
>ones I know how to fix), and IMHO is a lot easier to read.  The only 
>problem is that it doesn't work yet!  I'm getting a bunch of error 
>messages that scroll off the screen too quickly for me to read.  I'll 
>try to capture the error messages tomorrow using VMware and continue 
>debugging, but in the meantime I wanted to post what I have so far and 
>hopefully get some feedback and suggestions from the kernel hackers on 
>the list.  Here's the new source (I didn't try to create diffs: the 
>changes are too extensive).

Sounds good, I'll take a look if I get a chance.  I'm going on vacation
next week, tho, so don't hold your breath...


>
>http://anobject.com/jhamby/c99_kernel.tar.bz2
>
>Main highlights:
>
>* Compiles in C99 mode with few warnings and almost all warning
options 
>enabled.
>* uint8, uint16, uint32 are gone, replaced with uint8_t, uint16_t,
etc., 
>or one of these symbolic types:
>
>status_t  - most functions return this.. it's a uint_fast32_t
>count_t   - useful for loop counters and count variables
>flags_t   - used for variables holding flags or mode bits
>handle_t  - used to return a handle, when there wasn't already a type 
>for it, e.g. sem_id, area_id, etc.

All fine, in my opinion.

>
>* I've tried to follow a consistent pattern; most functions will have 
>one or more of these:
>
>status_t foo( struct Foo_s *psFoo )
>{
>       status_t nError = 0;
>
>       flags_t nFlg = cli();
>       sched_lock();
>
>       for ( count_t i = 0; i < psFoo->ff_nCount; ++i )
>       {
>               // do something
>               nError = ( -ENOMEM );
>       }
>
>       sched_unlock();
>       put_cpu_flags( nFlg );
>
>       return nError;
>}

I've locally modified the spin_lock() setup to automatically
save/restore interrupt masks, because it's almost always done that way,
but that can wait.
>

>/**
>  * Description of function.
>  * \param nFoo param 1
>  * \return the answer to life, the universe, and everything (42).
>  * \pre precondition
>  * \post postcondition
>  * \invariant pre- and post- and in-between condition :-)
>  */

I've added to this, and it might be generally useful:
 * \par Locks Required:
 * \par Locks Taken:

Then again, it might not, but the scheduler locks all over the place, so
I think it's usefull there.

>
>I also have a question for Daniel:  I would like to use the DLIST_ 
>macros in more places but unfortunately your version doesn't have a 
>pointer to the tail of the list so there's no quick way to append an 
>item to the end of a list or get the last item, operations which are 
>needed by most of the kernel's lists.  Do you have any plans for a new 
>version of dlist.h that might address this?  If you can add that 
>functionality to DLIST, then I'll take care of patching up the rest of 
>the kernel to use it.

It could definitely be done.  I have a version that does that locally.
However, there are two ways to do it, and I want an opinion on which to
use.  The first way is to just make the DLIST macros be a tail-queue.
The second is to introduce a second set of DLIST macros that are a
tail-queue, and keep the old ones unchanged.  The problem with a tail
queue is that you need the head pointer more often (in particular,
append and remove both need the head pointer in the tail-queue version,
but not in the current version).  Opinions?

Daniel