[Openvpn-devel] fix clang warning about missing braces

  From: Lev Stipakov <lev@openvpn.net>

A struct with subobjects should be initialized
with double braces.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpn/crypto.c         | 2 +-
 src/openvpn/crypto_mbedtls.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

On 28-11-2019 09:06, Lev Stipakov wrote:
> A struct with subobjects should be initialized
> with double braces.

This is not true. {0} is a valid initializer for structs in C. Both
clang and gcc used to have a bug where they incorrectly warned about
this. GCC fixed this a while ago[0]. I thought clang had fixed it too
recently, but apparently not?

-Steffan

[0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
  

Hi

On Thu, Nov 28, 2019 at 10:23 AM Steffan Karger <
steffan.karger@foxcrypto.com> wrote:

> On 28-11-2019 09:06, Lev Stipakov wrote:
> > A struct with subobjects should be initialized
> > with double braces.
>
> This is not true. {0} is a valid initializer for structs in C. Both
> clang and gcc used to have a bug where they incorrectly warned about
> this. GCC fixed this a while ago[0]. I thought clang had fixed it too
> recently, but apparently not?
>

Pretty much same as my thoughts.

I think the correct fix here is to remove -Werror from travis build.
I have tried and failed to lobby for this earlier, but one more try
can't hurt, I suppose :)

That said, it seems clang has fixed this some time after clang-7.
I don't get this warning anymore after upgrading to clang-9.

Selva
<div dir="ltr"><div>Hi<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Nov 28, 2019 at 10:23 AM Steffan Karger &lt;<a href="mailto:steffan.karger@foxcrypto.com" target="_blank">steffan.karger@foxcrypto.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 28-11-2019 09:06, Lev Stipakov wrote:<br>
&gt; A struct with subobjects should be initialized<br>
&gt; with double braces.<br>
<br>
This is not true. {0} is a valid initializer for structs in C. Both<br>
clang and gcc used to have a bug where they incorrectly warned about<br>
this. GCC fixed this a while ago[0]. I thought clang had fixed it too<br>
recently, but apparently not?<br></blockquote></div><div class="gmail_quote"><br></div><div class="gmail_quote">Pretty much same as my thoughts.<br></div><div class="gmail_quote"><br></div><div class="gmail_quote"><div>I think the correct fix here is to remove -Werror from travis build.</div><div>I have tried and failed to lobby for this earlier, but one more try</div><div>can&#39;t hurt, I suppose :)</div><div><br></div><div>That said, it seems clang has fixed this some time after clang-7.</div><div>I don&#39;t get this warning anymore after upgrading to clang-9.<br></div><div><br></div><div>Selva<br></div></div></div>
  

hi,

On Thu, Nov 28, 2019 at 11:56:34AM -0500, Selva Nair wrote:
> I think the correct fix here is to remove -Werror from travis build.
> I have tried and failed to lobby for this earlier, but one more try
> can't hurt, I suppose :)

Can we restrict -Werror to gcc builds, for the time being?

If not, I agree with you, and it needs to go...

Valid (and unambiguous) C statements shouldn't cause warnings, and we
shouldn't add patches just because an old version of a certain compiler
is silly.

gert
  

On 28/11/2019 20:43, Gert Doering wrote:
> hi,
> 
> On Thu, Nov 28, 2019 at 11:56:34AM -0500, Selva Nair wrote:
>> I think the correct fix here is to remove -Werror from travis build.
>> I have tried and failed to lobby for this earlier, but one more try
>> can't hurt, I suppose :)
> 
> Can we restrict -Werror to gcc builds, for the time being?
> 
> If not, I agree with you, and it needs to go...
> 
> Valid (and unambiguous) C statements shouldn't cause warnings, and we
> shouldn't add patches just because an old version of a certain compiler
> is silly.


With GCC-4.3.8, I see this warning:

crypto.c: In function ‘write_pem_key_file’:
crypto.c:1860:12: warning: missing braces around initializer [-Wmissing-braces]
     struct key server_key = { 0 };

Perhaps we should just add -Wno-missing-braces?  A quick patch is attached,
this silences this warning with GCC-4.3.8 at least.  That said, I'm not
sure this is the best approach; it may hide other missing braces warnings
we should see.
  

I upgraded clang to 9.0 and warning disappeared.

https://travis-ci.org/lstipakov/openvpn/builds/618527918

Patch is on the list.

to 28. marrask. 2019 klo 21.44 Gert Doering (gert@greenie.muc.de) kirjoitti:

> hi,
>
> On Thu, Nov 28, 2019 at 11:56:34AM -0500, Selva Nair wrote:
> > I think the correct fix here is to remove -Werror from travis build.
> > I have tried and failed to lobby for this earlier, but one more try
> > can't hurt, I suppose :)
>
> Can we restrict -Werror to gcc builds, for the time being?
>
> If not, I agree with you, and it needs to go...
>
> Valid (and unambiguous) C statements shouldn't cause warnings, and we
> shouldn't add patches just because an old version of a certain compiler
> is silly.
>
> gert
>
> --
> "If was one thing all people took for granted, was conviction that if you
>  feed honest figures into a computer, honest figures come out. Never
> doubted
>  it myself till I met a computer with a sense of humor."
>                              Robert A. Heinlein, The Moon is a Harsh
> Mistress
>
> Gert Doering - Munich, Germany
> gert@greenie.muc.de
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
  

Hi,

On Fri, Nov 29, 2019 at 11:47:02AM +0100, David Sommerseth wrote:
> With GCC-4.3.8, I see this warning:

This is about as old as you :-) - do we care about suppressing warnings
in old gcc versions that might suppress a *relevant* warning when
compiling with a current gcc version?

gert
  

On 29/11/2019 11:52, Gert Doering wrote:
> Hi,
> 
> On Fri, Nov 29, 2019 at 11:47:02AM +0100, David Sommerseth wrote:
>> With GCC-4.3.8, I see this warning:
> 
> This is about as old as you :-) - do we care about suppressing warnings
> in old gcc versions that might suppress a *relevant* warning when
> compiling with a current gcc version?

This is the default compiler on stock RHEL-7 ... which goes EOL August 2024.
  

Hi,

On Fri, Nov 29, 2019 at 12:25:13PM +0100, David Sommerseth wrote:
> On 29/11/2019 11:52, Gert Doering wrote:
> > On Fri, Nov 29, 2019 at 11:47:02AM +0100, David Sommerseth wrote:
> >> With GCC-4.3.8, I see this warning:
> > 
> > This is about as old as you :-) - do we care about suppressing warnings
> > in old gcc versions that might suppress a *relevant* warning when
> > compiling with a current gcc version?
> 
> This is the default compiler on stock RHEL-7 ... which goes EOL August 2024.

We're not going to break it.

I'm just not going to care very much about the warnings it might throw
(and I might object to any patches that are "just for the benefit of
suppressing warnings from gcc-4").

gert
  

On 29/11/2019 12:37, Gert Doering wrote:
> Hi,
> 
> On Fri, Nov 29, 2019 at 12:25:13PM +0100, David Sommerseth wrote:
>> On 29/11/2019 11:52, Gert Doering wrote:
>>> On Fri, Nov 29, 2019 at 11:47:02AM +0100, David Sommerseth wrote:
>>>> With GCC-4.3.8, I see this warning:
>>>
>>> This is about as old as you :-) - do we care about suppressing warnings
>>> in old gcc versions that might suppress a *relevant* warning when
>>> compiling with a current gcc version?
>>
>> This is the default compiler on stock RHEL-7 ... which goes EOL August 2024.
> 
> We're not going to break it.
> 
> I'm just not going to care very much about the warnings it might throw
> (and I might object to any patches that are "just for the benefit of
> suppressing warnings from gcc-4").

Fair enough.  But that will actually restrain us from adding -Werror by
default for some time forward, unless we want to make package maintenance a
bit more tricky - needing to revert a -Werror chage or add
-Wno-missing-braces.  If we have status quo for the moment, I can live with
that until RHEL-7 is EOL.
  

Hi,

On Fri, Nov 29, 2019 at 01:22:26PM +0100, David Sommerseth wrote:
> > I'm just not going to care very much about the warnings it might throw
> > (and I might object to any patches that are "just for the benefit of
> > suppressing warnings from gcc-4").
> 
> Fair enough.  But that will actually restrain us from adding -Werror by
> default for some time forward, unless we want to make package maintenance a
> bit more tricky - needing to revert a -Werror chage or add
> -Wno-missing-braces.  If we have status quo for the moment, I can live with
> that until RHEL-7 is EOL.

I don't think we can add -Werror as "on by default" for any platform
in the foreseeable future - we'll always hit "old" or "too new" compilers
that cause build breakage and lots of extra effort to work around it.

Now, well-defined environments ("travis, clang-9, Linux") I'm all for it :)

gert