Troy A. Griffitts wrote: > COLORED_ECHO([ BUILD TESTS: $enable_tests]) > COLORED_ECHO([ BUILD EXAMPLES: $enable_examples]) > COLORED_ECHO([ BUILD UTILITIES: $enable_utilities])
If we're going for comprehensiveness, we could add COLORED_ECHO([ CONF: $with_conf]) COLORED_ECHO([ PROFILEFN: $enable_profilefn]) COLORED_ECHO([ SHAREDLIB: $enable_shared]) COLORED_ECHO([ STATICLIB: $enable_static]) COLORED_ECHO([ WARNINGS: $enable_warnings]) as well. And then maybe sort them all alphabetically? > Of course we could look at these extra warnings and make some code > changes, but unless we build with these additional warnings always > enabled on our end, chances are, g++ will continue to add new warning > checks and we have the potential to release code which unknowingly won't > compile a debug build because we are adding -Werror with --enable-debug > > Thoughts on this? The -DFORTIFY_SOURCE=2 also adds various stack and buffer overflow checking, all manner of security goodness that I barely understand :) Apparently when first used on the Red Hat package codebase (in 2004) it found quite a few real security bugs. But that's past history. The warnings it generates on the current SWORD code are all related to not checking return values of 3 functions: read(), write() and fgets(). These tend to be mostly in the utilities, not the main library code itself, I think. My suspicion is that these low level areas of the SWORD codebase do not change all that fast, so a one time addition of patches to check these return values is sane and reasonable. At that point, whether to always check for such warnings or not, or have a separate --enable-fortify option to configure in order to see them, or something inbetween, isn't that big a deal for me. I see the security value in -D_FORTIFY_SOURCE=2, and my sense is that once the initial patch is in place, these checks won't bite anyone in SWORD for a very long time, so I'd be tempted to leave it enabled by default, but that's just me. Regarding -Werror, I'd recommend making --enable-debug do *only* what its help string in configure.ac says it does, which is "build debug library (default=no)". There is a separate configure option --enable-warnings which says it will "build with compiler warnings as errors (default=no)". In other words, it is really only --enable-warnings that should add -Werror to CXXFLAGS, not --enable-debug. If these two options worked independently, as their help strings seem to suggest they should, then there would be no real need (as I see it) for a default of -Werror anywhere. Those developers who need it for making *sure* there are no warnings left (just before a release, for example) can use it by design, at that time. Most people would by default build with plenty of warnings enabled, so those who care to look can see and fix them, but any extra warnings that arrive unexpectedly courtesy of g++ changes would not in and of themselves lead to a build failure (just to some new warnings appearing at build time). Best of both worlds? SUMMARY OF ABOVE SUGGESTIONS: (1) Patch the code for all the currently known warnings. (2) Make --enable-debug and --enable-warnings independent. (3) Default to -Wall -D_FORTIFY_SOURCE=2 for all gcc/g++ builds. (If (3) is too radical, then default to -Wall and add a new --enable-fortify option which adds -D_FORTIFY_SOURCE=2 . I just think that's more work for less benefit.) Jonathan _______________________________________________ sword-devel mailing list: sword-devel@crosswire.org http://www.crosswire.org/mailman/listinfo/sword-devel Instructions to unsubscribe/change your settings at above page
