On Sat, Oct 3, 2009 at 10:54 AM, <[email protected]> wrote: > Author: minfrin > Date: Sat Oct 3 14:54:00 2009 > New Revision: 821333 > > URL: http://svn.apache.org/viewvc?rev=821333&view=rev > Log: > mod_cache: Fix uri_meets_conditions() so that CacheEnable will > match by scheme, or by a wildcarded hostname. > PR: 40169 > Submitted by: Ryan Pendergast <rpender us.ibm.com> > Reviewed by: Graham Leggett > > Modified: > httpd/httpd/trunk/CHANGES > httpd/httpd/trunk/docs/manual/mod/mod_cache.xml > httpd/httpd/trunk/modules/cache/cache_util.c > > Modified: httpd/httpd/trunk/CHANGES > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=821333&r1=821332&r2=821333&view=diff > > ============================================================================== > --- httpd/httpd/trunk/CHANGES [utf-8] (original) > +++ httpd/httpd/trunk/CHANGES [utf-8] Sat Oct 3 14:54:00 2009 > @@ -10,6 +10,10 @@ > mod_proxy_ftp: NULL pointer dereference on error paths. > [Stefan Fritsch <sf fritsch.de>, Joe Orton] > > + *) mod_cache: Fix uri_meets_conditions() so that CacheEnable will > + match by scheme, or by a wildcarded hostname. PR 40169 > + [Ryan Pendergast <rpender us.ibm.com>, Graham Leggett] > + >
I guess it is "Submitted:" by both you and Ryan? (Commit log doesn't match CHANGES?) This is curious because this patch looks most like one attached to the issue by Peter Grandi. The last patch attached by Ryan is a one-liner, and now marked obsolete. > Modified: httpd/httpd/trunk/modules/cache/cache_util.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_util.c?rev=821333&r1=821332&r2=821333&view=diff > > ============================================================================== > --- httpd/httpd/trunk/modules/cache/cache_util.c (original) > +++ httpd/httpd/trunk/modules/cache/cache_util.c Sat Oct 3 14:54:00 2009 > some aspects of this patch change the style or handle certain issues that are ignored everywhere else > @@ -26,42 +26,79 @@ > > /* Determine if "url" matches the hostname, scheme and port and path > * in "filter". All but the path comparisons are case-insensitive. > + * Note: the 's' parameters is not used currently, but needed for > + * logging during debugging. > */ > -static int uri_meets_conditions(apr_uri_t filter, int pathlen, apr_uri_t > url) > -{ > - /* Compare the hostnames */ > - if(filter.hostname) { > - if (!url.hostname) { > +static int uri_meets_conditions(const server_rec * const s, > + const apr_uri_t filter, const int pathlen, const apr_uri_t url) { > + (void) s; > we have unused parameters all over the place (e.g., with hook functions); why we need to do "(void) s;" here? also, since no debug provision utilizing s was committed, the developer has to add code anyway when wanting to log something; can't they just add s at that time instead of leaving it in the code? it should take all of 15 seconds to do that + /* Is the filter is just for a local path or a proxy URI? */ > + if (!filter.scheme) { > + if (url.scheme || url.hostname) { > return 0; > } > - else if (strcasecmp(filter.hostname, url.hostname)) { > + } else { > I guess "} else {" is okay, even though 90% of the code doesn't use that construct. @@ -71,8 +108,7 @@ if (!url.path) { > if (*filter.path == '/' && pathlen == 1) { > return 1; > - } > - else { > + } else { > but it is even more odd to see that the commit modifies existing code to follow the rarely used style of the new code @@ -94,7 +130,7 @@ > for (i = 0; i < conf->cacheenable->nelts; i++) { > struct cache_enable *ent = > (struct cache_enable > *)conf->cacheenable->elts; > - if (uri_meets_conditions(ent[i].url, ent[i].pathlen, uri)) { > + if (uri_meets_conditions(r->server,ent[i].url, ent[i].pathlen, > uri)) { > more odd style
