[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Single Character Compile nit on Win32

From: Greg Stein <gstein_at_lyra.org>
Date: 2002-06-08 02:12:32 CEST

On Fri, Jun 07, 2002 at 04:06:36PM -0700, David Mankin wrote:
> On Fri, 7 Jun 2002, Zack Weinberg wrote:
>...
> > Well, as long as you're listening to me :-) you might want to try out

hehe...

> > a few more warning switches:
> >
> > -Wwrite-strings gives all string constants the type 'const char *' (as
> > they do in C++). This is liable to force you to add const qualifiers
> > all over the program, but will expose bugs that are otherwise pretty
> > much unfindable.

We already have this one :-)

(all these switches are set at line 210 of configure.in)

> > -Wstrict-prototypes makes sure every function declaration is a
> > prototype; -Wmissing-prototypes insists on a separate prototype for
> > every function definition. The combination of the two effectively
> > enforces a style where all functions are prototyped either at the top
> > of the file or in a header. Some people don't like this, but it's the
> > best way we've found to flush out type mismatches across files. (You

Hmm. Any function that is not 'static' is prototyped. However, the SVN style
does not prototype (internal) static function. If one of these are going to
start warning about non-prototyped static functions, then we can't use it.

Other than that, these switches sound good.

>...
> > -Wcast-qual will tell you if you ever cast away const or volatile.
> > This is not on by default because some people (including me) like to
> > have write-once structures in malloc space, referred to after creation
> > through const pointers, which eventually have to get freed -- and the
> > call to free() must cast away the const. (I'd be delighted to hear a
> > better way to approach this one.)

I saw this one, too, and it sounded good. It seems like I thought of a case
that wouldn't work well for us, but I don't remember. Given that we rarely
call free(), this might work okay.

We definitely have some long-needed cleanup in the FS with regards to skels
and const/non-const of the data stored in them (and links to other skels).
We'd probably need to do that work before turning this one on.

[ and the free() problem shouldn't bite us because of our pool usage ]

> > -W turns on a grab bag of warnings that have moderate to high
> > false-positive rates. Depending on your coding style, it may
> > be more trouble than it's worth. I don't know Subversion
> > style well enough to say.

While this has some interesting warnings, it would bite us too hard on the
"let unspecified struct members be initialized to zero" pattern. For
example, we do this quite often:

    struct foo bar = { 0 };

That zeroes out a struct quite nicely.

>...
> I haven't actually submitted any patches to Subversion and I have a
> little time to kill this weekend. I'll volunteer to turn on Zack's
> warnings and submit patches for problems they point out. Sounds like a
> bite-sized task, right?

Bite-sized or not... if it interests you, then go ahead. Philip and I
already cleaned out a few things after turning on -pedantic -Wno-long-long,
but I bet there are more things.

> I have gcc 2.95.2 on Mac OS X. Is that current enough to support the -W
> flags you're suggesting Zack? If not, I can upgrade my Apple Developer

Probably. Note that we shouldn't use flags that are "too new" unless we have
special checks for a recent GCC. That is, we could use some switches that
are pretty widely installed, and for the rest, then we check the version and
set them if we find GCC is recent enough.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jun 8 02:11:59 2002

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.