httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dean Gaudet <dgau...@arctic.org>
Subject const stuff (was Re: cvs commit: apache-1.3/src/modules/standard mod_log_referer.c)
Date Sun, 24 May 1998 22:48:41 GMT
Yeah I kind of figured this out later while I was thinking about it
offline.  Everyone else pointed out stuff that I would have -- Marc
mentioned the main problem, that you're modifying the referer result for
other callers.

Ben L. provided a const patch... I actually did this earlier this year,
but threw it away after I got about an hour into the work.  I figured you
all would complain to high-heaven if I made such a huge change.  My goal
was to make apache compile with gcc -Wwrite-strings cleanly.  Another
reason for dropping it was the reason Ben H. cited -- that it's hard to
nail it on all platforms.  .... but I could give it a try with apache-nspr
if folks are into this huge of a change.

Personally I think that all code should pass:

    -Wall -O -g     note that -O adds to what gcc can detect, and on
		    x86 at least, gcc is so poor that you can easily
		    debug -O code
    -fno-common
		    not all compilers do the bastardization that many
		    unix compilers do with "common"-style declarations
		    (think fortran).  In particular windows compilers
		    don't do this.
    -Wshadow
    -Wmissing-prototypes
		    we pass all of the above on Linux libc5 and glibc2
		    right now, all modules

The rest we don't quite pass yet:

    -Wpointer-arith
		    even better would be to just use -ansi and use
		    the __ form of things like __inline__, and
		    __attribute__

    -Wcast-qual
    -Wwrite-strings
		    we get nailed because of const issues like those
		    above

    -Wstrict-prototypes
		    right now the only reason we can't use this is
		    because of the lame function pointer without
		    prototype in command_rec -- the correct fix
		    can be implemented during an API overhaul.
		    I run with this occasionally and just filter
		    the http_config.h warning out.

    -Wnested-externs
		    Only .h files should declare externs -- everything
		    that is an exported interface should be declared
		    in a .h file.

Dean

On Wed, 20 May 1998, Brian Behlendorf wrote:

> 
> 
> On Wed, 20 May 1998, Dean Gaudet wrote:
> > On 20 May 1998 brian@hyperreal.org wrote:
> > 
> > >   @@ -175,7 +176,7 @@
> > >        referer = ap_table_get(orig->headers_in, "Referer");
> > >        if (referer != NULL) {
> > >    
> > >   -
> > >   +        ap_str_tolower(referer);
> > >            /* The following is an upsetting mess of pointers, I'm sorry
> > >               Anyone with the motiviation and/or the time should feel free
> > >               to make this cleaner... */
> > 
> > -1.  This patch is broken.  You cannot downcase the value you get from
> > table_get()... the values in all tables are constants.  You should replace
> > the to str_tolower()s with a single strcasecmp() or whatever.
> 
> The comparison is a strstr though; so far as I know there isn't a
> strcasestr available.  I suppose I can use regex... or just create a 
> new buffer.
> 
> Odd that the patch seemed to work fine, no compile errors and testing
> showed it did the job...
> 
> 	Brian
> 
> 
> 
> 


Mime
View raw message