Hi,

in modperl_filter.c my gcc gave a warning on this line:

            if (!init_handler->attrs & MP_FILTER_INIT_HANDLER) {
                Perl_croak(aTHX_ "handler %s doesn't have "
                           "the FilterInitHandler attribute set",
                           modperl_handler_name(init_handler));
            }

Without parentheses this is interpreted as

            if ((!init_handler->attrs) & MP_FILTER_INIT_HANDLER) {

MP_FILTER_INIT_HANDLER is a constant, 0x8. So, the expression can be reduced 
to "if (0)" or "if (1)" depending upon the numeric value of 1==1. I think this 
is not what is meant.

Now, if I change the if to what I think is meant:

            if (!(init_handler->attrs & MP_FILTER_INIT_HANDLER)) {

the test server won't even start:

handler TestFilter::in_init_basic::suicide_init doesn't have the 
FilterInitHandler attribute set.

This is correct, the attrs field is 0.

I think the culprit is the modperl_handler_new_from_sv function. It does not 
set the attrs field.

Any objections against this patch?

Index: src/modules/perl/modperl_handler.c                                       
                                    
===================================================================             
                                    
--- src/modules/perl/modperl_handler.c  (revision 930668)                       
                                    
+++ src/modules/perl/modperl_handler.c  (working copy)                          
                                    
@@ -497,6 +497,7 @@                                                             
                                    
 {                                                                              
                                    
     char *name = NULL;                                                         
                                    
     GV *gv;                                                                    
                                    
+    modperl_handler_t *handler;                                                
                                    
                                                                                
                                    
     if (SvROK(sv)) {                                                           
                                    
         sv = SvRV(sv);                                                         
                                    
@@ -515,7 +516,10 @@
             Perl_croak(aTHX_ "can't resolve the code reference");
         }
         name = apr_pstrcat(p, HvNAME(GvSTASH(gv)), "::", GvNAME(gv), NULL);
-        return modperl_handler_new(p, apr_pstrdup(p, name));
+       handler = modperl_handler_new(p, name);
+       handler->attrs=*modperl_code_attrs(aTHX_ (CV*)sv);
+
+       return handler;
       default:
         break;
     };
Index: src/modules/perl/modperl_filter.c
===================================================================
--- src/modules/perl/modperl_filter.c   (revision 930668)
+++ src/modules/perl/modperl_filter.c   (working copy)
@@ -407,7 +407,9 @@
             MP_TRACE_h(MP_FUNC, "found init handler %s",
                        modperl_handler_name(init_handler));

-            if (!init_handler->attrs & MP_FILTER_INIT_HANDLER) {
+           warn("init_attrs(%s)=%x\n", modperl_handler_name(init_handler),
+                init_handler->attrs);
+            if (!(init_handler->attrs & MP_FILTER_INIT_HANDLER)) {
                 Perl_croak(aTHX_ "handler %s doesn't have "
                            "the FilterInitHandler attribute set",
                            modperl_handler_name(init_handler));
@@ -656,6 +658,10 @@
     (void)SvUPGRADE(buffer, SVt_PV);
     SvPOK_only(buffer);
     SvCUR(buffer) = 0;
+    SvGROW(buffer, 1);         /* make PERL_ARGS_ASSERT_SV_CATPVN_FLAGS
+                                * happy, see Perl_sv_catpvn_flags() in sv.c
+                                * of perl 5.10.1
+                                */

     /* sometimes the EOS bucket arrives in the same brigade with other
      * buckets, so that particular read() will not return 0 and will

The SvGROW thing was necessary due to perl 5.10.1 as the comment explains. It 
is not related to the attributes problem. I can commit it in a separate step.

The patch also eliminates the useless apr_strdup in

        name = apr_pstrcat(p, HvNAME(GvSTASH(gv)), "::", GvNAME(gv), NULL);
        return modperl_handler_new(p, apr_pstrdup(p, name));

Torsten Förtsch

-- 
Need professional modperl support? Hire me! (http://foertsch.name)

Like fantasy? http://kabatinte.net
Index: src/modules/perl/modperl_handler.c
===================================================================
--- src/modules/perl/modperl_handler.c	(revision 930668)
+++ src/modules/perl/modperl_handler.c	(working copy)
@@ -497,6 +497,7 @@
 {
     char *name = NULL;
     GV *gv;
+    modperl_handler_t *handler;
 
     if (SvROK(sv)) {
         sv = SvRV(sv);
@@ -515,7 +516,10 @@
             Perl_croak(aTHX_ "can't resolve the code reference");
         }
         name = apr_pstrcat(p, HvNAME(GvSTASH(gv)), "::", GvNAME(gv), NULL);
-        return modperl_handler_new(p, apr_pstrdup(p, name));
+	handler = modperl_handler_new(p, name);
+	handler->attrs=*modperl_code_attrs(aTHX_ (CV*)sv);
+
+	return handler;
       default:
         break;
     };
Index: src/modules/perl/modperl_filter.c
===================================================================
--- src/modules/perl/modperl_filter.c	(revision 930668)
+++ src/modules/perl/modperl_filter.c	(working copy)
@@ -407,7 +407,9 @@
             MP_TRACE_h(MP_FUNC, "found init handler %s",
                        modperl_handler_name(init_handler));
 
-            if (!init_handler->attrs & MP_FILTER_INIT_HANDLER) {
+	    warn("init_attrs(%s)=%x\n", modperl_handler_name(init_handler),
+		 init_handler->attrs);
+            if (!(init_handler->attrs & MP_FILTER_INIT_HANDLER)) {
                 Perl_croak(aTHX_ "handler %s doesn't have "
                            "the FilterInitHandler attribute set",
                            modperl_handler_name(init_handler));
@@ -656,6 +658,10 @@
     (void)SvUPGRADE(buffer, SVt_PV);
     SvPOK_only(buffer);
     SvCUR(buffer) = 0;
+    SvGROW(buffer, 1);		/* make PERL_ARGS_ASSERT_SV_CATPVN_FLAGS
+				 * happy, see Perl_sv_catpvn_flags() in sv.c
+				 * of perl 5.10.1
+				 */
 
     /* sometimes the EOS bucket arrives in the same brigade with other
      * buckets, so that particular read() will not return 0 and will

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

Reply via email to