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