You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2006/05/18 10:50:05 UTC
Re: svn commit: r407357 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_storage.c
On Wed, May 17, 2006 at 07:16:43PM -0000, rpluem@apache.org wrote:
> --- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
> +++ httpd/httpd/trunk/modules/cache/cache_storage.c Wed May 17 12:16:43 2006
...
> @@ -375,15 +380,18 @@
> }
> }
> else {
> - scheme = "http";
> + scheme = ap_http_scheme(r);
> }
cache_storage.c: In function `cache_generate_key_default':
cache_storage.c:383: warning: assignment discards qualifiers from pointer target type
The below fixes the warning but I've no test setup for the cache handy,
does it still work? (the lower-casing code just above here could be
simplified like this too)
Index: modules/cache/cache_storage.c
===================================================================
--- modules/cache/cache_storage.c (revision 407503)
+++ modules/cache/cache_storage.c (working copy)
@@ -320,8 +320,8 @@
apr_status_t cache_generate_key_default(request_rec *r, apr_pool_t* p,
char**key)
{
- char *port_str, *scheme, *hn;
- const char * hostname;
+ char *port_str, *hn;
+ const char *hostname, *scheme;
int i;
/*
@@ -373,11 +373,10 @@
* manner (see above why this is needed).
*/
if (r->proxyreq && r->parsed_uri.scheme) {
- /* Copy the scheme */
- scheme = apr_pcalloc(p, strlen(r->parsed_uri.scheme) + 1);
- for (i = 0; r->parsed_uri.scheme[i]; i++) {
- scheme[i] = apr_tolower(r->parsed_uri.scheme[i]);
- }
+ /* Copy the scheme and lower-case it. */
+ char *lcs = apr_pstrdup(p, r->parsed_uri.scheme);
+ ap_str_tolower(lcs);
+ scheme = lcs;
}
else {
scheme = ap_http_scheme(r);
Re: svn commit: r407357 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_storage.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/22/2006 04:49 PM, Joe Orton wrote:
>
> Great, thanks. In reply to your other questions: "casts are bad" :),
> and I don't think that performance is a good excuse for randomly
> manually inlining functions either (sometimes overall performance is
> improved by reducing executable size since more code can then be cached
> in any case). You could already pick cycle-counting holes in the
> inlined code too; e.g. use of pcalloc is redundant since only the NUL
> terminator is preserved.
Thanks for explanation. You convinced me :-).
Fixed in r408729.
Regards
Rüdiger
Re: svn commit: r407357 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_storage.c
Posted by Joe Orton <jo...@redhat.com>.
On Sun, May 21, 2006 at 12:31:00PM +0200, Ruediger Pluem wrote:
> On 05/18/2006 10:50 AM, Joe Orton wrote:
> > cache_storage.c: In function `cache_generate_key_default':
> > cache_storage.c:383: warning: assignment discards qualifiers from pointer target type
>
> Fixed in r408154. Thanks.
Great, thanks. In reply to your other questions: "casts are bad" :),
and I don't think that performance is a good excuse for randomly
manually inlining functions either (sometimes overall performance is
improved by reducing executable size since more code can then be cached
in any case). You could already pick cycle-counting holes in the
inlined code too; e.g. use of pcalloc is redundant since only the NUL
terminator is preserved.
Regards,
joe
Re: svn commit: r407357 - in /httpd/httpd/trunk: CHANGES modules/cache/cache_storage.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/18/2006 10:50 AM, Joe Orton wrote:
> On Wed, May 17, 2006 at 07:16:43PM -0000,
>
>>--- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
>>+++ httpd/httpd/trunk/modules/cache/cache_storage.c Wed May 17 12:16:43 2006
>
> ...
>
>>@@ -375,15 +380,18 @@
>> }
>> }
>> else {
>>- scheme = "http";
>>+ scheme = ap_http_scheme(r);
>> }
>
>
> cache_storage.c: In function `cache_generate_key_default':
> cache_storage.c:383: warning: assignment discards qualifiers from pointer target type
Fixed in r408154. Thanks.
Regards
Rüdiger