Xorg coding style (Was: Re: Radeon TV-in support in Xorg CVS.)

[Keith Packard]

Around 21 o'clock on Oct 3, "Ronny V. Vindenes" wrote:

> However one big obstacle remains; there is little or no consistency in
> code throughout the project.

Given that the project started 17 years ago, it's not a huge surprise that 
various style variations have been tried in different parts of the code.  
Other projects have had the benefit of consistent leadership, which X has 
not.

The following is purely personal opinion and not intended to serve as a 
set of requirements for coding styles.  But, I will continue to use the 
described styles in code that I author, so you might as well get used to 
seeing it...

In the historical sections of the code base (Xserver, lib/X11), there are 
two main styles which differ only in how braces are applied.  Both use 4 
space indentation with a mixture of tab and spaces used to line up the 
text.  Howver, tab stops are *always* on 8 character boundaries.

lib/X11:

    if (foo) {
	some code;
    } else {
	some more code;
    }

Xserver:

    if (foo)
    {
	some code;
    }
    else
    {
	some more code;
    }

I'm OK with either varient, with a slight preference to the separate 
braces (having worked mostly in Xserver).  It does encourage single 
statement code blocks to avoid braces, which is plus/minus on readability.

Any decent editor should be able to manage mixing tabs and spaces to get 4 
spaces between levels (even vi can manage with 'set shift-width 4').

I've seen the no-indent style in various embedded code written by hardware 
engineers; I'll take the whitespace, thanks-very-much.  Using only four 
spaces instead of 8 leaves plenty of room inside the function body while 
still making it easy to see the function on the screen.  If you're running
out of room and wrapping many lines, it may be better to split your
statements up; temporary variables offer an opportunity to describe 
intermediate values.

I also have a habit of aligning function formals and locals:

void
fbCopyWindowProc (DrawablePtr   pSrcDrawable,
                  DrawablePtr   pDstDrawable,
                  GCPtr         pGC,
                  BoxPtr        pbox,
                  int           nbox,
                  int           dx,
                  int           dy,
                  Bool          reverse,
                  Bool          upsidedown,
                  Pixel         bitplane,
                  void          *closure)
{
    FbBits      *src;
    FbStride    srcStride;
    int         srcBpp;
    int         srcXoff, srcYoff;
    FbBits      *dst;
    FbStride    dstStride;
    int         dstBpp;
    int         dstXoff, dstYoff;

...
}

I haven't found an editor which helps do this automatically, so I do spend 
more time than I'd like realigning things when changing parameters or 
adding new locals.

I use a lot of whitespace to try and separate code into paragraphs which 
describe one complete idea

Some parts of DIX indent switch cases an extra 2 spaces:

	switch (foo)
	{
	  case Fooish:
	    some code;
	    break;
	  case Barish
	    more code;
	    break;
	}

I prefer the switch cases to align under the switch.  I also make an 
exception for this case and place the brace on the line with the switch 
because the first 'case' provides ample white space to find the block of 
code:

	switch (foo) {
	case Fooish:
	    some code;
	    break;
	case Barish:
	    more code;
	    break;
	}

Having said all of the above, I think the most important thing to do while
editing code is to adopt the existing style and not patch your own style
into a large body of existing code.  If the existing style on some long
abandoned is so horrific or inconsistenly applied as to make working on the
code painful, I'd much rather see a pure whitespace patch applied before
any functional changes are made.

I don't know if 'diff -w' can validate a whitespace change which includes
newlines for correctness or not; that would certainly be a useful test if
it does work.

For code which has active developers, I suggest that those people should 
get to decide on a coding style, about the only requirement that I feel we 
should require is that it use 8 space tabstops for '\t' characters.

I do have to agree a bit with Ronny on Vladimir's particular style -- it's 
one of the hardest I've ever seen to parse quickly.  It seems optimized to 
avoid keystrokes in a particularily lame text editor; either vim or emacs 
can produce code as shown above with a very similar number of keystrokes.
(except for my aligned formals and locals, which I'll admit are something 
of a compulsion for me).  But, I'll take hard-to-read code over /dev/null 
for most things...

-keith


-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg/attachments/20041003/7419e88a/attachment.pgp>