You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Jean-Frederic <jf...@gmail.com> on 2007/02/26 09:40:06 UTC
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
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