Avoid some new gcc warnings in 15 #10808

pull TheBlueMatt wants to merge 2 commits into bitcoin:master from TheBlueMatt:2017-07-15-new-warnings changing 2 files +2 −2

TheBlueMatt commented at 6:47 PM on July 12, 2017: member

This, plus #10714 avoids adding new compile-time warnings in 15.

Fixes a warning added in #10792.
Fix signed/unsigned comparison warning introduced in c8e29d7ff0. 1016dacfac
gmaxwell approved
gmaxwell commented at 7:52 PM on July 12, 2017: contributor

utACK
meshcollider commented at 11:46 PM on July 12, 2017: contributor

utACK
achow101 commented at 11:53 PM on July 12, 2017: member

utACK a8a602eeece2cfd847a361127fe7f41b26f13c26
dcousens approved
in src/prevector.h:225 in a8a602eeec outdated
```
 219 | @@ -220,7 +220,10 @@ class prevector {
 220 |          }
 221 |      }
 222 |  
 223 | -    prevector() : _size(0) {}
 224 | +    prevector() : _size(0) {
 225 | +        // Explicitly initialize indirect ptr to avoid "used uninitialized" warning
 226 | +        _union.indirect = nullptr;
```
sipa commented at 12:52 AM on July 13, 2017:

I was vaguely concerned that this assignment violates type punning through a union, but (1) some sources say that is permitted anyway (2) the other type is char (for which exceptions to type punning exist) and (3) we'll write to the union again before ever using data in it.

laanwj commented at 6:12 PM on July 13, 2017:

I though type-punning through an union was the only acceptable way to do it?

theuni commented at 6:38 PM on July 13, 2017:

From IRC: <cfields> sipa: why not use aggregate initialization: prevector() : _size(0), _union{{}} <cfields> or give the union a ctor?

I can't reproduce the warning, so I can't be sure that it satisfies whatever compiler is complaining, but the above compiles fine for me, and I'd think that initializing via a ctor would work otherwise.

gmaxwell commented at 11:31 PM on July 13, 2017:

@laanwj There is a faction of language wonks/compiler folks that argues that it is never acceptable (except with char) per the language specs; -- and I believe there is nothing in any of the C++ standards that make it clear that its kosher (though sipa tells me apparently C11 has something explicit). Since having no way to do it is not a very realistic position it doesn't reflect the behavior of compilers today. Generally better to avoid it at least where its convenient, or at least be aware that someday it might become non-kosher depending on which evil spirits control the souls of the compiler authors next week. :)

TheBlueMatt commented at 3:13 PM on July 14, 2017:

I'm super not convinced this is an issue. If we're really concerned, easiest fix is probably require C11 and turn prevector into C++ wrapper around C. More realistically, gcc explicitly supports this, and I believe many other compilers as well.
sipa commented at 12:52 AM on July 13, 2017: member

utACK
paveljanik commented at 8:39 AM on July 13, 2017: contributor

ACK

Please fix typo in the second commit message (initizlie).
sipa commented at 5:17 PM on July 14, 2017: member

@theuni's suggestion looks the cleanest to me. Would you mind trying that?
TheBlueMatt force-pushed on Jul 14, 2017
TheBlueMatt commented at 8:31 PM on July 14, 2017: member

OK, took @theuni's version (which has no warnings for me).

Explicitly initialize prevector::_union to avoid new warning

Warning from gcc 7.1 is ./prevector.h:450:25: warning:
'*((void*)(&<anonymous>)+8).prevector<28, unsigned char>::_union.prevector<28, unsigned char>::direct_or_indirect::<anonymous>.prevector<28, unsigned char>::direct_or_indirect::<unnamed struct>::indirect'
may be used uninitialized in this function [-Wmaybe-uninitialized]

c73b8be244

TheBlueMatt force-pushed on Jul 15, 2017
sipa commented at 12:53 AM on July 15, 2017: member

utACK c73b8be2440e424f304fda22e43240e8b90f9b00
sipa merged this on Jul 15, 2017
sipa closed this on Jul 15, 2017
sipa referenced this in commit ec8a50b8d7 on Jul 15, 2017
PastaPastaPasta referenced this in commit a49d864673 on Jul 6, 2019
PastaPastaPasta referenced this in commit 790aa75608 on Jul 8, 2019
PastaPastaPasta referenced this in commit e12f8f4e78 on Jul 9, 2019
PastaPastaPasta referenced this in commit 5b1e542770 on Jul 11, 2019
PastaPastaPasta referenced this in commit 68ac27486c on Jul 13, 2019
PastaPastaPasta referenced this in commit ac6162afd5 on Jul 17, 2019
PastaPastaPasta referenced this in commit 354b75f390 on Jul 17, 2019
PastaPastaPasta referenced this in commit 29760e78bc on Jul 18, 2019
barrystyle referenced this in commit 8ab4cfaa33 on Jan 22, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#10714 Avoid printing incorrect block indexing time due to uninitialized variable #10792 Replace MAX_OPCODE for OP_NOP10.