perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip M. Gollucci" <pgollu...@p6m7g8.com>
Subject Re: simplify mod_perl.c a bit
Date Sun, 04 Apr 2010 01:39:45 GMT
What ANSI C std is that?

perl core code and fbsd code do this all over the place.  They are
supposedly C-99.




On 4/2/2010 4:40 PM, Philippe M. Chiasson wrote:
> On 10-03-31 11:07 , Torsten Förtsch wrote:
>> Hi,
>>
>> the patch below simplifies mod_perl.c a bit.
>>
>> Instead of
>>
>> modperl_response_handler_run(r, finish) {
>>   do something
>>   if( finish ) {
>>     modperl_response_finish()
>>   }
>> }
>>
>> and calling that function in one place as
>>
>> modperl_response_handler_run(r, TRUE)
>>
>> and in the second place as
>>
>> modperl_response_handler_run(r, TRUE)
>> do something
>> modperl_response_finish()
>>
>> it now looks like
>>
>> modperl_response_handler_run(r) {
>>   do something
>> }
>>
>> 1st usage:
>> modperl_response_handler_run(r)
>> modperl_response_finish()
>>
>> 2nd usage:
>> modperl_response_handler_run(r)
>> do something
>> modperl_response_finish()
> 
> Nice, thanks, and a good idea. Functions that take some flag only to
> trigger different behaviour are usually bad form. Below is a slightly
> different patch do address a simple nit.
> 
> In ANSI C you can't define variables in blocks, like
> 
> int foo(void) {
>   int a = 2;
>   some code
>   {
>     int b = 3;
>     more code
>   }
> 
> So the below patch fixes that by making the rc status variable global
> to the function. I compile with -Wall -Werror and it tends to pick these
> up.
> 
> Nice side-effect is that it ends up simplifying a little further the
> case where calling modperl_response_finish() is necessary.
> 
> Otherwise, +1
> 
> Index: src/modules/perl/mod_perl.c
> ===================================================================
> --- src/modules/perl/mod_perl.c (revision 930351)
> +++ src/modules/perl/mod_perl.c (working copy)
> @@ -991,7 +991,7 @@
>      return modperl_wbucket_flush(rcfg->wbucket, FALSE);
>  }
> 
> -static int modperl_response_handler_run(request_rec *r, int finish)
> +static int modperl_response_handler_run(request_rec *r)
>  {
>      int retval;
> 
> @@ -1003,13 +1003,6 @@
>          r->handler = r->content_type; /* let http_core or whatever try */
>      }
> 
> -    if (finish) {
> -        apr_status_t rc = modperl_response_finish(r);
> -        if (rc != APR_SUCCESS) {
> -            retval = rc;
> -        }
> -    }
> -
>      return retval;
>  }
> 
> @@ -1019,7 +1012,7 @@
>  #ifdef USE_ITHREADS
>      MP_dRCFG;
>  #endif
> -    apr_status_t retval;
> +    apr_status_t retval, rc;
> 
>  #ifdef USE_ITHREADS
>      pTHX;
> @@ -1043,7 +1036,11 @@
>          modperl_env_request_populate(aTHX_ r);
>      }
> 
> -    retval = modperl_response_handler_run(r, TRUE);
> +    retval = modperl_response_handler_run(r);
> +    rc = modperl_response_finish(r);
> +    if (rc != APR_SUCCESS) {
> +        retval = rc;
> +    }
> 
>  #ifdef USE_ITHREADS
>      if (MpInterpPUTBACK(interp)) {
> @@ -1099,7 +1096,7 @@
> 
>      modperl_env_request_tie(aTHX_ r);
> 
> -    retval = modperl_response_handler_run(r, FALSE);
> +    retval = modperl_response_handler_run(r);
> 
>      modperl_env_request_untie(aTHX_ r);
> 


-- 
------------------------------------------------------------------------
1024D/DB9B8C1C B90B FBC3 A3A1 C71A 8E70  3F8C 75B8 8FFB DB9B 8C1C
Philip M. Gollucci (pgollucci@p6m7g8.com) c: 703.336.9354
VP Apache Infrastructure; Member, Apache Software Foundation
Committer,                        FreeBSD Foundation
Sr. System Admin,                 Ridecharge Inc.
Consultant,                       P6M7G8 Inc.

Work like you don't need the money,
love like you'll never get hurt,
and dance like nobody's watching.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Mime
View raw message