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 2005/10/12 12:09:28 UTC

Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c

On Tue, Oct 11, 2005 at 09:43:35PM -0000, Jim Jagielski wrote:
...
> Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?rev=312964&r1=312963&r2=312964&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Tue Oct 11 14:43:32 2005
> @@ -1212,24 +1212,34 @@
>                                                    const char *url)
>  {
>      proxy_worker *worker;
> -    char *c, *uri = apr_pstrdup(p, url);
> +    proxy_worker *max_worker = NULL;
> +    int max_match = 0;
> +    int url_length;
> +    int worker_name_length;
> +    char *c;
>      int i;
>  
> -    c = strchr(uri, ':');
> +    c = strchr(url, ':');
>      if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
>         return NULL;

That broke the -Wall -Werror build with --enable-maintainer-mode

cc1: warnings being treated as errors
proxy_util.c: In function 'ap_proxy_get_worker':
proxy_util.c:1222: warning: passing argument 1 of 'ap_strchr' discards qualifiers from pointer target type

is this the code which is about to be removed anyway?

joe

Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c

Posted by Rüdiger Plüm <r....@gmx.de>.

On 10/12/2005 01:38 PM, Joe Orton wrote:

[..cut..]

> 
> 
> Yup, if using strchr on a const variable, the correct method to preserve 
> const-ness and avoid those warnings is:
> 
>   const char *foo;
> 
>   foo = ap_strchr_c(some_const_char_string, 'x');
> 
> note that 'c' would need to be const above too.

Thanks for that info. It took me a while to understand the macros
in util_debug.c / httpd.h (small letter macros OtherBill
just mentioned to dislike :-)).

I think I understand now what is going on and that in DEBUG mode
the strict checking whether const or not is enforced by wrapper
functions whereas in the non debug mode it is a simple macro that
maps things directly to the libc functions which do not do this strict
checking sometimes.
If I understand Jim correctly we should use ap_strchr / ap_strchr_c
throughout the code, but it is not done currently. Maybe I have a look
into this on a rainy day and exchange strchr against ap_strchr / ap_strchr_c
where appropriate :-).


Regards

Rüdiger




Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Oct 12, 2005 at 12:53:32PM +0200, Ruediger Pluem wrote:
> 
> 
> On 10/12/2005 12:09 PM, Joe Orton wrote:
> > On Tue, Oct 11, 2005 at 09:43:35PM -0000, Jim Jagielski wrote:
> > ...
> > 
> >>Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
> >>URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?rev=312964&r1=312963&r2=312964&view=diff
> >>==============================================================================
> >>--- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
> >>+++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Tue Oct 11 14:43:32 2005
> >>@@ -1212,24 +1212,34 @@
> >>                                                   const char *url)
> >> {
> >>     proxy_worker *worker;
> >>-    char *c, *uri = apr_pstrdup(p, url);
> >>+    proxy_worker *max_worker = NULL;
> >>+    int max_match = 0;
> >>+    int url_length;
> >>+    int worker_name_length;
> >>+    char *c;
> >>     int i;
> >> 
> >>-    c = strchr(uri, ':');
> >>+    c = strchr(url, ':');
> >>     if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
> >>        return NULL;
> > 
> > 
> > That broke the -Wall -Werror build with --enable-maintainer-mode
> > 
> > cc1: warnings being treated as errors
> > proxy_util.c: In function 'ap_proxy_get_worker':
> > proxy_util.c:1222: warning: passing argument 1 of 'ap_strchr' discards qualifiers from pointer target type
> 
> Sorry for the stupid question: Is this because *url is const?

Yup, if using strchr on a const variable, the correct method to preserve 
const-ness and avoid those warnings is:

  const char *foo;

  foo = ap_strchr_c(some_const_char_string, 'x');

note that 'c' would need to be const above too.

joe

Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 10/12/2005 12:09 PM, Joe Orton wrote:
> On Tue, Oct 11, 2005 at 09:43:35PM -0000, Jim Jagielski wrote:
> ...
> 
>>Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
>>URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c?rev=312964&r1=312963&r2=312964&view=diff
>>==============================================================================
>>--- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
>>+++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Tue Oct 11 14:43:32 2005
>>@@ -1212,24 +1212,34 @@
>>                                                   const char *url)
>> {
>>     proxy_worker *worker;
>>-    char *c, *uri = apr_pstrdup(p, url);
>>+    proxy_worker *max_worker = NULL;
>>+    int max_match = 0;
>>+    int url_length;
>>+    int worker_name_length;
>>+    char *c;
>>     int i;
>> 
>>-    c = strchr(uri, ':');
>>+    c = strchr(url, ':');
>>     if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
>>        return NULL;
> 
> 
> That broke the -Wall -Werror build with --enable-maintainer-mode
> 
> cc1: warnings being treated as errors
> proxy_util.c: In function 'ap_proxy_get_worker':
> proxy_util.c:1222: warning: passing argument 1 of 'ap_strchr' discards qualifiers from pointer target type

Sorry for the stupid question: Is this because *url is const?

> 
> is this the code which is about to be removed anyway?

Yes, I currently discuss this with Jim. The question is if this check is a valuable quick way or not.
I don't think that this is the case in a correctly configured system, but I would like to wait
for Jims response on that.

Regards

Rüdiger

Re: svn commit: r312964 - in /httpd/httpd/branches/2.2.x: CHANGES modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c

Posted by Jim Jagielski <ji...@jaguNET.com>.
On Oct 12, 2005, at 6:09 AM, Joe Orton wrote:

> On Tue, Oct 11, 2005 at 09:43:35PM -0000, Jim Jagielski wrote:
> ...
>
>> Modified: httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c
>> URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.2.x/ 
>> modules/proxy/proxy_util.c?rev=312964&r1=312963&r2=312964&view=diff
>> ===================================================================== 
>> =========
>> --- httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c (original)
>> +++ httpd/httpd/branches/2.2.x/modules/proxy/proxy_util.c Tue Oct  
>> 11 14:43:32 2005
>> @@ -1212,24 +1212,34 @@
>>                                                    const char *url)
>>  {
>>      proxy_worker *worker;
>> -    char *c, *uri = apr_pstrdup(p, url);
>> +    proxy_worker *max_worker = NULL;
>> +    int max_match = 0;
>> +    int url_length;
>> +    int worker_name_length;
>> +    char *c;
>>      int i;
>>
>> -    c = strchr(uri, ':');
>> +    c = strchr(url, ':');
>>      if (c == NULL || c[1] != '/' || c[2] != '/' || c[3] == '\0')
>>         return NULL;
>>
>
> That broke the -Wall -Werror build with --enable-maintainer-mode
>
> cc1: warnings being treated as errors
> proxy_util.c: In function 'ap_proxy_get_worker':
> proxy_util.c:1222: warning: passing argument 1 of 'ap_strchr'  
> discards qualifiers from pointer target type
>
> is this the code which is about to be removed anyway?
>

Argf. Yeah, that should be fixed. Of course, we should also, esp
in the the 2.3/4 trunk and the 2.3/4 branch standardize all our
usage of strchr. It's current a mix and mess of all flavors :)

I'll fix this later on today.