perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Torsten Förtsch <torsten.foert...@gmx.net>
Subject what appeared to be a small typo
Date Tue, 06 Apr 2010 10:51:11 GMT
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

Mime
View raw message