You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Paul Querna <ch...@force-elite.com> on 2006/08/04 07:19:20 UTC
Re: svn commit: r427920 - /httpd/httpd/trunk/modules/proxy/mod_proxy.c
Please review the code style guidelines:
http://httpd.apache.org/dev/styleguide.html
In the following revisions I found things that I consider to not be in
line with the guidelines:
427920
427928
428008
428029
Specifically, most of them are missing { and } around one life if/else
statements.
Comments inline bellow...
mturk@apache.org wrote:
> Author: mturk
> Date: Tue Aug 1 23:54:01 2006
> New Revision: 427920
....
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=427920&r1=427919&r2=427920&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Tue Aug 1 23:54:01 2006
> @@ -1605,6 +1605,7 @@
> proxy_balancer *balancer = NULL;
> proxy_worker *worker = NULL;
> const char *err;
> + int in_proxy_section = 0;
>
> if (cmd->directive->parent &&
> strncasecmp(cmd->directive->parent->directive,
> @@ -1616,6 +1617,7 @@
> name = ap_getword_conf(cmd->temp_pool, &pargs);
> if ((word = ap_strchr(name, '>')))
> *word = '\0';
> + in_proxy_section = 1;
> }
> else {
> /* Standard set directive with worker/balancer
> @@ -1627,15 +1629,32 @@
> if (strncasecmp(name, "balancer:", 9) == 0) {
> balancer = ap_proxy_get_balancer(cmd->pool, conf, name);
> if (!balancer) {
> - return apr_pstrcat(cmd->temp_pool, "ProxySet can not find '",
> - name, "' Balancer.", NULL);
> + if (in_proxy_section) {
> + err = ap_proxy_add_balancer(&balancer,
> + cmd->pool,
> + conf, name);
> + if (err)
> + return apr_pstrcat(cmd->temp_pool, "ProxySet ",
> + err, NULL);
For example, this code block should be:
if (err) {
return apr_pstrcat(cmd->temp_pool, "ProxySet ",
err, NULL);
}
> + }
> + else
> + return apr_pstrcat(cmd->temp_pool, "ProxySet can not find '",
> + name, "' Balancer.", NULL);
And this should be:
}
else {
return apr_pstrcat(cmd->temp_pool, "ProxySet can not find '",
name, "' Balancer.", NULL);
}
Thanks,
Paul