httpd-test-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1)" <madhusudan_mathiha...@hp.com>
Subject RE: [PATCH] Style police for mod_specweb99.c
Date Wed, 22 Jan 2003 19:36:36 GMT
I wonder how I missed the first one :-?.. Anyways, thanks for the careful
review.. (Ah.. now I know - I did a "cb" before I did the expand", and cb
must have been the culprit)..

As regards the function names at the end of the function, I sure like it -
it's a lot easier when you browse through the code.. But then, I thought it
should be there for all of them or for none of them.. That's the reason I
decided to take it away.. The next patch will have it..

-Madhu

-----Original Message-----
From: Greg Ames [mailto:gregames@apache.org]
Sent: Wednesday, January 22, 2003 11:26 AM
To: test-dev@httpd.apache.org
Subject: Re: [PATCH] Style police for mod_specweb99.c


MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1) wrote:
> Summary of the patch :
> 1. convert tabs to spaces
> 2. try to follow apache styleguide

Overall, a big improvement...thanks much!

nitpicky comments follow:

> --- mod_specweb99.c	15 Jan 2003 16:15:02 -0000	1.18
> +++ mod_specweb99.c	21 Jan 2003 21:39:14 -0000

>  static apr_int16_t
>  getCADFile(struct server_rec *sv, struct request_rec *r,
> -         struct specweb99_module_data * _my); 
> +struct specweb99_module_data * _my);

ooops...the function name should be on the same line with the 
return type, and 
the struct arg should line up with the one above it.

> +        ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
> +                     "mod_specweb: ap_pass_brigade failed 
for buffer '%s'",
> +                     buf);
>      }
> -}				/* returnHTMLPageWithBuffer */
> +}

I prefer keeping the name on the end of a the function which is 
over a page 
long.  But the tabs gotta go.

Now I see why there are still tabs.  expand -i only gets rid of 
leading tabs - 
too conservative.

>      }
> -}				/* returnHTMLPageWithFile */

same here.

>  static void specweb99_child_init(apr_pool_t * p, server_rec *s)
>  {

> +         * (e.g. think ~user and other redirect cases with 
clobber the
> +         * concept of a docroot).
> +         */
>      };
> -}				/* specweb99_module_init */
> +}

yikes! this one was the wrong label - an apparent 
cut'n'paste-o.  OK, it goes.

>      returnHTMLPageWithBuffer(r, buf, len);
>  
> -}				/* customadscan */
> +}

this one should stay, minus the tabs (long function).

Greg

Mime
View raw message