tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jean-Frederic <jfcl...@gmail.com>
Subject Re: svn commit: r511326 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_map.c common/jk_util.c
Date Mon, 26 Feb 2007 08:40:06 GMT
On Sun, 2007-02-25 at 17:34 +0100, Rainer Jung wrote:
> Two more questions:
> 
> jfclere@apache.org schrieb:
> > Modified: tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c
> > URL:
> http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c?view=diff&rev=511326&r1=511325&r2=511326
> >
> ==============================================================================
> > --- tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c (original)
> > +++ tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c Sat Feb 24
> 11:02:40 2007
> > @@ -1744,9 +1744,9 @@
> >      jk_server_conf_t *conf =
> >          (jk_server_conf_t *) ap_get_module_config(s->module_config,
> >                                                    &jk_module);
> > -
> > + 
> >      if (jk_map_read_property(conf->worker_properties, line, 1,
> conf->log) == JK_FALSE)
> > -        return ap_pstrcat(cmd->temp_pool, "Invalid JkWorkerProperty
> ", line);
> > +        return ap_pstrcat(cmd->temp_pool, "Invalid JkWorkerProperty
> ", line, NULL);
> >  
> >      return NULL;
> >  }
> > @@ -2543,8 +2543,9 @@
> >              ap_log_error(APLOG_MARK, APLOG_EMERG, s,
> >                           "No worker file and no worker options in
> httpd.conf "
> >                           "use JkWorkerFile to set workers");
> > -            return;
> >          }
> > +        ap_log_error(APLOG_MARK, APLOG_EMERG | APLOG_NOERRNO, 0,
> NULL, "Error in reading worker properties");
> > +        return !OK;
> 
> This is inside jk_init which is void. I think this is correct w.r.t
> the
> apache 1.3 module structure.

Fixed.

> 
> 
> > Modified: tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c
> > URL:
> http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c?view=diff&rev=511326&r1=511325&r2=511326
> >
> ==============================================================================
> > --- tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c (original)
> > +++ tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c Sat Feb 24
> 11:02:40 2007
> > @@ -2664,12 +2664,14 @@
> >                           0, NULL,
> >                           "No worker file and no worker options in
> httpd.conf"
> >                           "use JkWorkerFile to set workers");
> > -            return;
> >          }
> > +        ap_log_error(APLOG_MARK, APLOG_EMERG | APLOG_NOERRNO, 0,
> NULL, "Error in reading worker properties");
> > +        return !OK;
> >      }
> >  
> >      if (jk_map_resolve_references(init_map, "worker.", 1, 1,
> conf->log) == JK_FALSE) {
> > -        jk_error_exit(APLOG_MARK, APLOG_EMERG, s, pconf, "Error in
> resolving configuration references");
> > +        ap_log_error(APLOG_MARK, APLOG_EMERG | APLOG_NOERRNO, 0,
> NULL, "Error in resolving configuration references");
> > +        return !OK;
> >      }
> >  
> >      /* we add the URI->WORKER MAP since workers using AJP14
> > @@ -2701,7 +2704,7 @@
> >                                        pconf)) != APR_SUCCESS) {
> >          ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
> >                       "mod_jk: could not create jk_log_lock");
> > -        return HTTP_INTERNAL_SERVER_ERROR;
> > +        return !OK;
> >      }
> >  
> >  #if JK_NEED_SET_MUTEX_PERMS
> > @@ -2710,7 +2713,7 @@
> >          ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
> >                       "mod_jk: Could not set permissions on "
> >                       "jk_log_lock; check User and Group
> directives");
> > -        return HTTP_INTERNAL_SERVER_ERROR;
> > +        return !OK;
> >      }
> >  #endif
> >  
> > @@ -2728,7 +2731,7 @@
> >                  jk_server_conf_t *sconf = (jk_server_conf_t
> *)ap_get_module_config(srv->module_config,
> >
> &jk_module);
> >                  if (open_jklog(srv, pconf))
> > -                    return HTTP_INTERNAL_SERVER_ERROR;
> > +                    return !OK;
> >                  if (sconf) {
> >                      if (!uri_worker_map_alloc(&(sconf->uw_map),
> >
> sconf->uri_to_context, sconf->log))
> > @@ -2776,7 +2779,8 @@
> >                      }
> >                  }
> >              }
> > -            init_jk(pconf, conf, s);
> > +            if (init_jk(pconf, conf, s))
> > +                return !OK;
> >          }
> >      }
> 
> Apache 2.0/2.2 says, you can either return OK or DECLINED, everything
> else is an error. So returning !OK means depending on an
> implementation
> detail of apache return constants (!OK could be equal to DECLINED). I
> csn see, that the construct !OK is also used in apache code itself,
> but
> only in three modules.
> 
> I would prefer to return another code, that's known to be different
> form
> OK and DECLINED. If you insist on súsing !OK, there's still one
> "return
> HTTP_INTERNAL_SERVER_ERROR;" left in line 2766.

Ok it seems using HTTP_INTERNAL_SERVER_ERROR is a better idea.
I have fixed it.

Cheers

Jean-Frederic

> 
> Regards,
> 
> Rainer 


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


Mime
View raw message