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);
--
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
signature.asc
Description: OpenPGP digital signature
