You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Robert S. Thau" <rs...@ai.mit.edu> on 1996/07/01 18:55:54 UTC

I *don't* want Paul's style guide.

  Revised style guide. I've explicitly listed the indentation rules which may
  or may not be contentious. 

Perhaps I'm missing British understatement again, but I saw no such
notations anywhere in your draft.

  I've admitted defeat on the function declaration
  issue and fixed up all I could spot and made the few other changes that
  I remember people noting. 

So you *were* deliberately dragging your heels on the clear outcome
of a vote to try to impose your preferences on the rest of the group,
even after the nominal basis of those preferences ("you can grep for
the definitions") was shown to be of little merit (you can grep for
them anyway).

  I may have missed a few, lots of mail when I get
  in on Mondays because I'm usually away at weekends.

You did.  In fact, you missed some which had been clearly addressed to
you well before the weekend, viz. both enums and function-declaration
style, which you *still* haven't got right, despite repeated
corrections.  If you wanted to do a decent and conscientious job as
redactor, you should have been saving all this mail, or at least have
gone through the archive after the fact.  See below.

I'm afraid these things do not raise my confidence in you as redactor
of this document.  At this point, I'd offer to do it myself, but I
simply haven't got the time --- I don't even have the time to do even
critiques as cursory as the one below; I shall now have to explain to
my advisor why I was writing it instead of chapter six of my thesis.

However, there are a *lot* of recommendations in the KNF document
which do not reflect existing consensus, many of which I frankly
regard as bad practice (i.e. "don't initialize your variables", "don't
declare variables in blocks").  In fact, the only thing in that
document which I think I agree with is brace-style for control
structures.  I would also therefore prefer to see the document either
written from scratch or based on some document which is closer to what
the group's actual preferences *are*.  

Starting from a list of winning entries on Ben's ballots will at least
avoid dragging in stuff on which no consensus was ever reached.  (Ben,
if you'd like to do things this way --- you seem to be putting in a
lot of time on style matters anyway --- you'd have my full support,
for what little it seems to be worth around here).

In any case, here are the things that leapt out at me in reviewing
Paul's current attempt.  Paul, if you do another one, I would expect
to see all of these either dealt with, or moved to a separate, clearly
delineated section of contentious items.

  /*
   * Multi-line comments look like this.  Make them real sentences.  Fill
   * them so they look like real paragraphs.
   */

/* My multi-line comments generally look like this.  Is there
 * any consensus on the above form?
 */

/****************************************************************
 *
 * As a sole exception, I will sometimes head a *group* of related
 * functions in a file by a "section divider" like this.
 */

  /* Enum types are capitalized. */
  enum enumtype { ONE, TWO } et;

As noted elsewhere, they aren't in existing code, and I don't much
like the practice.

  /*
   *
   * Major structures should be declared at the top of the file they are
   * used in, or in separate header files, if they are used in multiple
   * source files. Use of the structures should be by separate declarations
   * and should be "extern" if they are declared in a header file.
   */

I'm not sure what a "major" structure is.  At any rate, when a
particular structure is used only by a given routine or set of
routines in one file, I think it is by far the best practice to put
the declaration with them, to keep it close to the use.  

(As a side note, the formatting of this comment does not conform to
your own guidelines above).

  /*
   * All major routines should have a comment briefly describing what
   * they do. The comment before the "main" routine should describe
   * what the program does.
   */

"Major".  There's that word again.  I note, BTW, that the example code
following is indented eight spaces...

	/*
	 * Space after keywords (while, for, return, switch).  No braces are
	 * used for single statement block.
	 *
	 * Forever loops are done with for's, not while's.
	 */

This I agree with, actually... but it does not seem to reflect group
consensus.

	for (;;)
		stmt;
	
	/*
	 * Parts of a for loop may be left empty.  Avoid declarations in
	 * blocks unless the routine is unusually complicated.
	 */

Agree with the first, disagree violently with the second.  If a variable
is used in only one block, it ought to be declared in that block, to 
minimize head-scratching about possible uses elsewhere.

	/*
	 * Try to put shorter part first.  The closing and opening braces
	 * go on the same line as the else.
	 */

Do they?  There's a vote out on this.


   /*
    * Example of function declaration style.
    */
   static char *function(int a1, int a2, float a3, int a4)
   {

You *still* haven't got this right.  The opening brace goes on the
same line as everything else, if there's room for it.

	/*
	 * If a line overflows reuse the type keyword.
	 * In general, don't initialize variables in the declarations.
	 */

Again, I disagree strongly with the second of these recommendations
--- I regard initialization of variables in the declarations, where
possible, to be good style (so long as initializations are kept one
to a line, and not admixed with declarations of uninitialized variables; 
they shouldn't be easy to miss).

	/*
         *  Also, test pointers
	 * against NULL, i.e. use:
	 *
	 * 	(p = f()) == NULL
	 * not:
	 *	!(p = f())
         */

!foo is used all over the current Apache code.

rst

Re: I *don't* want Paul's style guide.

Posted by Paul Richards <p....@elsevier.co.uk>.
Ben Laurie writes:
 > > In particular what happens when sizeof(char *) > sizeof(int)
 > > e.g. Cray or Digital UNIX, or when NULL != (void *)0 (maybe on some
 > > microsoft systems) ?
 > 
 > Also quoting K&R: "The symbolic constant NULL is often used in place of zero".

Yeah but the next line says

"The symbolic constant NULL is often used in place of zero, as a mnemonic
to indicate *more clearly* that this is a special value for a pointer"

emphasis is obviously mine :-)

Just above that it says "Pointers and integers are not interchangeable. Zero
is the sole exception...", which is why using NULL is good practice to
clearly show it's a pointer and not an integer that's being used.

A null pointer is definately zero, from the Appendix in K&R

A6.6

An integral constant expression with value 0, or such an expression cast to
type void *, may be converted, by a cast, by assignment, or by comparison,
to a pointer of any type. This produces a null pointer that is equal to another
null pointer of the same type, but unequal to any pointer to a function or
object.


Re: I *don't* want Paul's style guide.

Posted by David Robinson <dr...@esi.co.uk>.
On Mon, 1 Jul 1996, Robert S. Thau wrote:
> 	/*
>          *  Also, test pointers
> 	 * against NULL, i.e. use:
> 	 *
> 	 * 	(p = f()) == NULL
> 	 * not:
> 	 *	!(p = f())
>          */
> !foo is used all over the current Apache code.

This is one that really irritates me (perhaps almost as much as my
comment style irritates rst 8-( ) because to my mind, '!' takes a boolean
argument, not a pointer. (Reasons below.)

But, amazingly, I _can_ actually understand what is meant here.
Am I the only person who thinks that this discussion has turned into a
complete waste of time?

 David.

Why (!p) is wrong
-----------------

As I understand C,
char *p;

 if (!p) ...

is equivalent to
 if (!(int)p) ...

Which seems to me to mean something quite different to
 if (p != NULL) ...

In particular what happens when sizeof(char *) > sizeof(int)
e.g. Cray or Digital UNIX, or when NULL != (void *)0 (maybe on some
microsoft systems) ?

Even if in practice the two test are likely to evaulate to the same
answer, it does seem like sloppy programming to me.



Re: I *don't* want Paul's style guide.

Posted by Alexei Kosut <ak...@organic.com>.
On Mon, 1 Jul 1996, Robert S. Thau wrote:

>   Revised style guide. I've explicitly listed the indentation rules which may
>   or may not be contentious. 
> 
> Perhaps I'm missing British understatement again, but I saw no such
> notations anywhere in your draft.

There was something that said 4, I think.

> However, there are a *lot* of recommendations in the KNF document
> which do not reflect existing consensus, many of which I frankly
> regard as bad practice (i.e. "don't initialize your variables", "don't
> declare variables in blocks").  In fact, the only thing in that
> document which I think I agree with is brace-style for control
> structures.  I would also therefore prefer to see the document either
> written from scratch or based on some document which is closer to what
> the group's actual preferences *are*.  

Sounds good to me.

> Starting from a list of winning entries on Ben's ballots will at least
> avoid dragging in stuff on which no consensus was ever reached.  (Ben,
> if you'd like to do things this way --- you seem to be putting in a
> lot of time on style matters anyway --- you'd have my full support,
> for what little it seems to be worth around here).

I dunno... Ben seemed pretty far removed from the consensus. I think the
"winner" should write it :)

[...]

>   /*
>    * Multi-line comments look like this.  Make them real sentences.  Fill
>    * them so they look like real paragraphs.
>    */
> 
> /* My multi-line comments generally look like this.  Is there
>  * any consensus on the above form?
>  */

I perfer the latter for short multi-line and the former for long
multi-line.

> I'm not sure what a "major" structure is.  At any rate, when a
> particular structure is used only by a given routine or set of
> routines in one file, I think it is by far the best practice to put
> the declaration with them, to keep it close to the use.  

Hmm. This one I'm not so sure about. With the current Apache code, which
follows this rule for the most part, it can get really annoying when you'd
really like to call a function in http_protocol.c from a module, but you
can't, because there's no header file that has it. I suppose on one hand
it encourages writing code that would work on any Apache API-compatible
server (not that there are others), but it can get annoying...

> 	/*
> 	 * Parts of a for loop may be left empty.  Avoid declarations in
> 	 * blocks unless the routine is unusually complicated.
> 	 */
> 
> Agree with the first, disagree violently with the second.  If a variable
> is used in only one block, it ought to be declared in that block, to 
> minimize head-scratching about possible uses elsewhere.

Agreed.

> 	/*
> 	 * Try to put shorter part first.  The closing and opening braces
> 	 * go on the same line as the else.
> 	 */
> 
> Do they?  There's a vote out on this.

I don't think so. (at least, *I* don't think so)

>    /*
>     * Example of function declaration style.
>     */
>    static char *function(int a1, int a2, float a3, int a4)
>    {
> 
> You *still* haven't got this right.  The opening brace goes on the
> same line as everything else, if there's room for it.

Actually, I think for function declarations we had voted to have the brace
on the next line.

> 	/*
> 	 * If a line overflows reuse the type keyword.
> 	 * In general, don't initialize variables in the declarations.
> 	 */
> 
> Again, I disagree strongly with the second of these recommendations
> --- I regard initialization of variables in the declarations, where
> possible, to be good style (so long as initializations are kept one
> to a line, and not admixed with declarations of uninitialized variables; 
> they shouldn't be easy to miss).

Agreed.

> 	/*
>          *  Also, test pointers
> 	 * against NULL, i.e. use:
> 	 *
> 	 * 	(p = f()) == NULL
> 	 * not:
> 	 *	!(p = f())
>          */
> 
> !foo is used all over the current Apache code.

Defenitely. I personally would like someone to explain to me why you'd
ever want to test something against NULL, instead of just letting the
expression evaluate itself. Correct me if I'm wrong, but NULL is usually
defined as either (void*)0 or 0, and either way, it's just 0,
which is the same thing that !foo evaluates to if foo is not 0. And, IMO,
!foo looks a lot cleaner (and shorter).

-- Alexei Kosut <ak...@organic.com>            The Apache HTTP Server 
   http://www.nueva.pvt.k12.ca.us/~akosut/      http://www.apache.org/