Re: svn commit: r329849 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c

[Jim Jagielski <ji...@jaguNET.com>]

I wanted to avoid making string copies when possible. Plus, we
don't want to lowercase the URL, since that means /Foo/bar
would be the same as /FOO/Bar, which is wrong :)

Ruediger Pluem wrote:
> 
> 
> 
> On 10/31/2005 05:31 PM, jim@apache.org wrote:
> > Author: jim
> > Date: Mon Oct 31 08:31:29 2005
> > New Revision: 329849
> > 
> > URL: http://svn.apache.org/viewcvs?rev=329849&view=rev
> > Log:
> > Fix a problem where we are doing a case insensitive
> > match between the worker and the URL. Instead, only
> > the scheme and hostname are insensitive, the rest
> > should be case sensitive.
> > 
> > Modified:
> >     httpd/httpd/trunk/CHANGES
> >     httpd/httpd/trunk/modules/proxy/proxy_util.c
> 
> 
> Thanks for looking into this. I think this is also related to PR36906.
> 
> Given the fact that the hostname and the schema already get lowercased by
> ap_proxy_add_worker, wouldn't it be faster and clearer to do something like
> the following (honest question, I do not know the answer and the best code should
> go in):
> 
> 
> PROXY_DECLARE(proxy_worker *) ap_proxy_get_worker(apr_pool_t *p,
>                                                   proxy_server_conf *conf,
>                                                   const char *url)
> {
>     proxy_worker *worker;
>     proxy_worker *max_worker = NULL;
>     int max_match = 0;
>     int url_length;
>     int worker_name_length;
>     const char *c;
>     int i;
>     char *url_copy;
> 
>     c = ap_strchr_c(url, ':');
>     if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
>        return NULL;
> 
>     url_copy = apr_pstrdup(p, url);
>     url_length = strlen(url_copy);
> 
>     /*
>      * We need to find the start of the path and
>      * and lowercase all characters before.
>      */
>     c = ap_strchr_c(c+3, '/');
>     if (c) {
>         *c = '\0';
>         ap_str_tolower(url_copy);
>         *c = '/';
>     }
>     else {
>         ap_str_tolower(url_copy);
>     }
> 
>     worker = (proxy_worker *)conf->workers->elts;
> 
>     /*
>      * Do a "longest match" on the worker name to find the worker that
>      * fits best to the URL.
>      */
>     for (i = 0; i < conf->workers->nelts; i++) {
>         if ( ((worker_name_length = strlen(worker->name)) <= url_length)
>            && (worker_name_length > max_match)
>            && (strncmp(url_copy, worker->name, worker_name_length) == 0)) {
>             max_worker = worker;
>             max_match = worker_name_length;
>         }
>         worker++;
>     }
>     return max_worker;
> }
> 
> Regards
> 
> Rüdiger
> 


-- 
=======================================================================
 Jim Jagielski   [|]   jim@jaguNET.com   [|]   http://www.jaguNET.com/
           "If you can dodge a wrench, you can dodge a ball."

Re: svn commit: r329849 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c

[Ruediger Pluem <rp...@apache.org>]

On 11/01/2005 02:05 PM, Jim Jagielski wrote:
> I wanted to avoid making string copies when possible. Plus, we

Ok. That was one point of my question as I saw the need to copy the
string as a drawback compared to your approach. I was only unsure how
much this counts.

> don't want to lowercase the URL, since that means /Foo/bar
> would be the same as /FOO/Bar, which is wrong :)

Sure, but I actually do not do this. I only lowercase scheme and hostname, because
I temporarily terminate my copied string with \0 before I lower case it. So from
the functional point of view my approach should do the same as yours. The question
was more about which solution

- is faster
- is more easy to understand if both versions are equally fast

Regards

Rüdiger

[..cut..]