Re: [Openvpn-devel] [PATCH v2] Enable stricter compiler warnings by default

2018-01-31 Thread Gert Doering
Hi,

On Thu, Feb 01, 2018 at 07:34:44AM +0100, Steffan Karger wrote:
> > Just one thing: if we support compilers other than gcc/clang (say,
> > xlc) this would need a test like AX_CHECK_COMPILE_FLAG().
> 
> Hm, I don't think we do for configure.ac.  We do sort-of support MSVC,
> but that doesn't use this build system.  Gert, David, any opinions?

Well, on systems that have no gcc, configure will use whatever it finds
- on Solaris this could be some Sun CC, on AIX it will be xlc.  We should
not break that gratuitously.

So, gcc-specific flags need a test on whether they work with "OS cc"

gert
-- 
now what should I write here...

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Enable stricter compiler warnings by default

2018-01-31 Thread Steffan Karger
Hi,

On 27-01-18 17:47, Selva Nair wrote:
> As this is added by default (not in response to --enable/--disable
> directives), may be better to prepend to user's CFLAGS? I'm not sure
> what the standard practice is, though.. Considering the complexity of
> configury, preserving user's input is hard, so may be ok like this.

Good point.  I'll send a v3 with CFLAGS last (for default flags).

> Just one thing: if we support compilers other than gcc/clang (say,
> xlc) this would need a test like AX_CHECK_COMPILE_FLAG().

Hm, I don't think we do for configure.ac.  We do sort-of support MSVC,
but that doesn't use this build system.  Gert, David, any opinions?

-Steffan

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v2] Enable stricter compiler warnings by default

2018-01-27 Thread Selva Nair
Hi,

On Fri, Jan 26, 2018 at 5:58 AM, Steffan Karger  wrote:
> This by default enables the compiler warnings one could previously
> enable using the --enable-strict configure option.  I think it is
> okay to do so now, because we've taken care of many warnings in the
> more standard builds.  (Most of those were totally harmless, but they
> prevented us from spotting new more serious mistakes.)
>
> The --enable-strict flag now enables two extra warning flags that I
> think can be useful:
>
> -Wsign-compare warns when the compiler promotes a signed type to
> unsigned before comparing, which can lead to unexpected behaviour.
>
> -Wuninitialized adds extra warnings about usage of uninitialized variables
> or struct elements.
>
> Signed-off-by: Steffan Karger 
> ---
> v2: Just move forward with warnings, don't add --disable-strict.
>
>  configure.ac | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 2c1937e5..4d415565 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1292,13 +1292,14 @@ if test "${enable_pkcs11}" = "yes"; then
> )
>  fi
>
> +CFLAGS="${CFLAGS} -Wall -Wno-unused-parameter -Wno-unused-function"

As this is added by default (not in response to --enable/--disable
directives), may be better to prepend to user's CFLAGS? I'm not sure
what the standard practice is, though.. Considering the complexity of
configury, preserving user's input is hard, so may be ok like this.

>  if test "${enable_pedantic}" = "yes"; then
> enable_strict="yes"
> CFLAGS="${CFLAGS} -pedantic"
> AC_DEFINE([PEDANTIC], [1], [Enable pedantic mode])
>  fi
>  if test "${enable_strict}" = "yes"; then
> -   CFLAGS="${CFLAGS} -Wall -Wno-unused-parameter -Wno-unused-function"
> +   CFLAGS="${CFLAGS} -Wsign-compare -Wuninitialized"

I'm no fan of -Wsign-compare, but it does warn against some sloppy
coding, is not on by default, so fine by me.

And, I very much like -Wall -Wno-unused-foo becoming the "new normal".

Just one thing: if we support compilers other than gcc/clang (say,
xlc) this would need a test like AX_CHECK_COMPILE_FLAG(). Otherwise
looks good.

Selva

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v2] Enable stricter compiler warnings by default

2018-01-26 Thread Steffan Karger
This by default enables the compiler warnings one could previously
enable using the --enable-strict configure option.  I think it is
okay to do so now, because we've taken care of many warnings in the
more standard builds.  (Most of those were totally harmless, but they
prevented us from spotting new more serious mistakes.)

The --enable-strict flag now enables two extra warning flags that I
think can be useful:

-Wsign-compare warns when the compiler promotes a signed type to
unsigned before comparing, which can lead to unexpected behaviour.

-Wuninitialized adds extra warnings about usage of uninitialized variables
or struct elements.

Signed-off-by: Steffan Karger 
---
v2: Just move forward with warnings, don't add --disable-strict.

 configure.ac | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 2c1937e5..4d415565 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1292,13 +1292,14 @@ if test "${enable_pkcs11}" = "yes"; then
)
 fi
 
+CFLAGS="${CFLAGS} -Wall -Wno-unused-parameter -Wno-unused-function"
 if test "${enable_pedantic}" = "yes"; then
enable_strict="yes"
CFLAGS="${CFLAGS} -pedantic"
AC_DEFINE([PEDANTIC], [1], [Enable pedantic mode])
 fi
 if test "${enable_strict}" = "yes"; then
-   CFLAGS="${CFLAGS} -Wall -Wno-unused-parameter -Wno-unused-function"
+   CFLAGS="${CFLAGS} -Wsign-compare -Wuninitialized"
 fi
 if test "${enable_werror}" = "yes"; then
CFLAGS="${CFLAGS} -Werror"
-- 
2.14.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel