[Openvpn-devel] Enable extra compiler warnings by default

Message ID 20180120164114.12478-1-steffan@karger.me
State Superseded
Headers show
Series
  • [Openvpn-devel] Enable extra compiler warnings by default
Related show

Commit Message

Steffan Karger Jan. 20, 2018, 4:41 p.m.
This by default enables the compiler warnings any 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.)

Those who prefer a more silent build can use --disable-strict to revert
to the old default behaviour.

Signed-off-by: Steffan Karger <steffan@karger.me>
---
 configure.ac | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Selva Nair Jan. 21, 2018, 5:37 a.m. | #1
Hi,

On Sat, Jan 20, 2018 at 11:41 AM, Steffan Karger <steffan@karger.me> wrote:
>
> This by default enables the compiler warnings any 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.)
>
> Those who prefer a more silent build can use --disable-strict to revert
> to the old default behaviour.

I think --enable-strict by default is good to have. In fact its needed to
keep on bugging us about many of those printf format spec issues we
still have with mingw.

>
>
> Signed-off-by: Steffan Karger <steffan@karger.me>
> ---
>  configure.ac | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 2c1937e5..2b5fa533 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -209,9 +209,9 @@ AC_ARG_ENABLE(
>
>  AC_ARG_ENABLE(
>         [strict],
> -       [AS_HELP_STRING([--enable-strict], [enable strict compiler warnings (debugging option) @<:@default=no@:>@])],
> +       [AS_HELP_STRING([--disable-strict], [enable strict compiler warnings (debugging option) @<:@default=yes@:>@])],

The description should now read "disable strict compiler warnings...",
isn't it? Could be fixed while merging..

>
>         ,
> -       [enable_strict="no"]
> +       [enable_strict="yes"]
>  )
>
>  AC_ARG_ENABLE(

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Steffan Karger Jan. 21, 2018, 9:17 a.m. | #2
Hi,

On 21-01-18 06:37, Selva Nair wrote:
> 
> On Sat, Jan 20, 2018 at 11:41 AM, Steffan Karger <steffan@karger.me> wrote:
>>
>> This by default enables the compiler warnings any 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.)
>>
>> Those who prefer a more silent build can use --disable-strict to revert
>> to the old default behaviour.
> 
> I think --enable-strict by default is good to have. In fact its needed to
> keep on bugging us about many of those printf format spec issues we
> still have with mingw.
> 
>>
>> Signed-off-by: Steffan Karger <steffan@karger.me>
>> ---
>>  configure.ac | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 2c1937e5..2b5fa533 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -209,9 +209,9 @@ AC_ARG_ENABLE(
>>
>>  AC_ARG_ENABLE(
>>         [strict],
>> -       [AS_HELP_STRING([--enable-strict], [enable strict compiler warnings (debugging option) @<:@default=no@:>@])],
>> +       [AS_HELP_STRING([--disable-strict], [enable strict compiler warnings (debugging option) @<:@default=yes@:>@])],
> 
> The description should now read "disable strict compiler warnings...",
> isn't it? Could be fixed while merging..
> 

Yeah, it should.  Though while brushing my teeth last night, I was
wondering whether it's worth to have --disable-strict at all.  Shouldn't
we just unconditionally enable the current strict compiler flags?  We
could then make the --enable-strict flags a bit stricter to catch more
mistakes.

-Steffan

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Selva Nair Jan. 26, 2018, 1:08 a.m. | #3
Hi,

On Sun, Jan 21, 2018 at 4:17 AM, Steffan Karger <steffan@karger.me> wrote:
> Hi,
>
> On 21-01-18 06:37, Selva Nair wrote:
>>
>> On Sat, Jan 20, 2018 at 11:41 AM, Steffan Karger <steffan@karger.me> wrote:
>>>
>>> This by default enables the compiler warnings any 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.)
>>>
>>> Those who prefer a more silent build can use --disable-strict to revert
>>> to the old default behaviour.
>>
>> I think --enable-strict by default is good to have. In fact its needed to
>> keep on bugging us about many of those printf format spec issues we
>> still have with mingw.
>>
>>>
>>> Signed-off-by: Steffan Karger <steffan@karger.me>
>>> ---
>>>  configure.ac | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index 2c1937e5..2b5fa533 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -209,9 +209,9 @@ AC_ARG_ENABLE(
>>>
>>>  AC_ARG_ENABLE(
>>>         [strict],
>>> -       [AS_HELP_STRING([--enable-strict], [enable strict compiler warnings (debugging option) @<:@default=no@:>@])],
>>> +       [AS_HELP_STRING([--disable-strict], [enable strict compiler warnings (debugging option) @<:@default=yes@:>@])],
>>
>> The description should now read "disable strict compiler warnings...",
>> isn't it? Could be fixed while merging..
>>
>
> Yeah, it should.  Though while brushing my teeth last night, I was
> wondering whether it's worth to have --disable-strict at all.  Shouldn't
> we just unconditionally enable the current strict compiler flags?  We
> could then make the --enable-strict flags a bit stricter to catch more
> mistakes.

Personally either way would be great improvement as I forget to enable
-Wall almost every time and have to re-configure.

Selva

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Patch

diff --git a/configure.ac b/configure.ac
index 2c1937e5..2b5fa533 100644
--- a/configure.ac
+++ b/configure.ac
@@ -209,9 +209,9 @@  AC_ARG_ENABLE(
 
 AC_ARG_ENABLE(
 	[strict],
-	[AS_HELP_STRING([--enable-strict], [enable strict compiler warnings (debugging option) @<:@default=no@:>@])],
+	[AS_HELP_STRING([--disable-strict], [enable strict compiler warnings (debugging option) @<:@default=yes@:>@])],
 	,
-	[enable_strict="no"]
+	[enable_strict="yes"]
 )
 
 AC_ARG_ENABLE(