| Message ID | 20191128080631.117-1-lstipakov@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel] fix clang warning about missing braces | expand |
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 <<a href="mailto:steffan.karger@foxcrypto.com" target="_blank">steffan.karger@foxcrypto.com</a>> 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> > A struct with subobjects should be initialized<br> > 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'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'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
diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index 65e789ed..731f5bad 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -1857,7 +1857,7 @@ void write_pem_key_file(const char *filename, const char *pem_name) { struct gc_arena gc = gc_new(); - struct key server_key = { 0 }; + struct key server_key = {{ 0 }}; struct buffer server_key_buf = clear_buf(); struct buffer server_key_pem = clear_buf(); diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c index 3e77fa9e..57ad85ad 100644 --- a/src/openvpn/crypto_mbedtls.c +++ b/src/openvpn/crypto_mbedtls.c @@ -304,7 +304,7 @@ mbedtls_ctr_drbg_context * rand_ctx_get(void) { static mbedtls_entropy_context ec = {0}; - static mbedtls_ctr_drbg_context cd_ctx = {0}; + static mbedtls_ctr_drbg_context cd_ctx = {{0}}; static bool rand_initialised = false; if (!rand_initialised)