Trac issueshttps://gitlab.torproject.org/legacy/trac/-/issues2020-06-13T14:58:37Zhttps://gitlab.torproject.org/legacy/trac/-/issues/19378Remove completely irrelevant warnings from the clang list2020-06-13T14:58:37ZNick MathewsonRemove completely irrelevant warnings from the clang listSome of the warnings in configure.ac that I added as part of #19180 are probably c++-only, or objc-only, or openmp-only. We should disable those.Some of the warnings in configure.ac that I added as part of #19180 are probably c++-only, or objc-only, or openmp-only. We should disable those.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/19380Hand-audit compiler warning results which we wouldn't want to have on-by-defa...2020-06-13T14:58:38ZNick MathewsonHand-audit compiler warning results which we wouldn't want to have on-by-default.These warnings aren't definitely indicative of bugs in our code, and don't seem to be possible for us to fix in all cases. Still, it might be worth auditing all the cases where these warnings trigger, since they _might_ indicate bugs or...These warnings aren't definitely indicative of bugs in our code, and don't seem to be possible for us to fix in all cases. Still, it might be worth auditing all the cases where these warnings trigger, since they _might_ indicate bugs or possible areas of improvement.
```
strict-overflow=3...5 (4.2)
Behaves pretty differently on different GCC versions.
We get warnings for just about every case where we have pointer
math in an addition. That seems nutty.
padded (3)
Not a mistake. Worth looking over for hand-audit purposes, but mostly
harmless.
unsafe-loop-optimizations (4.1)
Worth hand-auditing, but triggers on every kind of interesting for loop.
covered-switch-default
Usually this is defensive programming, but it could be a mistake
in some cases, or could cover up future mistakes?
```Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/19220Do not override Makefile user variables2020-06-13T14:58:09ZcypherpunksDo not override Makefile user variablesThis was supposed to be a comment to #19216 but i figured it's better to open a new ticket because it's out of scope.
The real solution to the issue fixed in ticket:19216#comment:4 is to stop assigning to CFLAGS (unless when testing the...This was supposed to be a comment to #19216 but i figured it's better to open a new ticket because it's out of scope.
The real solution to the issue fixed in ticket:19216#comment:4 is to stop assigning to CFLAGS (unless when testing the flag but restore the old CFLAGS after) because it's a [user variable](https://www.gnu.org/software/autoconf/manual/automake.html#User-Variables) which should never be set by the package. Instead the additional parameters should be assigned to a temporary variable (like `TOR_CFLAGS` or something) which can be `AC_SUBST`ed and appended to `AM_CFLAGS`. The same can be said about `CPPFLAGS` and `LDFLAGS`. For more information see [https://www.gnu.org/software/autoconf/manual/automake.html#Flag-Variables-Ordering].
Overriding user variables also causes issues when trying to run `make distcheck` in build directories that are configured with `--enable-fatal-warnings`. It seems the overridden CFLAGS are integrated into the tarball and causes the default Autoconf checks to fail.Tor: unspecifiedhttps://gitlab.torproject.org/legacy/trac/-/issues/19379Consider adding even more compiler warnings, even when they require code chan...2020-06-13T14:58:37ZNick MathewsonConsider adding even more compiler warnings, even when they require code changes.Some of these warnings _might_ be worth adding, either globally or selectively, but they would require a significant amount of effort to modify our code. (I already got the easy cases with #19180, I think.)
```
cast-qual (4.6)
...Some of these warnings _might_ be worth adding, either globally or selectively, but they would require a significant amount of effort to modify our code. (I already got the easy cases with #19180, I think.)
```
cast-qual (4.6)
Rationale: triggers everywhere, even in some pretty normal C. Would
be nice to have it trigger less, but would need to blow up a bunch
of API things. Bigger project.
conversion (4.6)
Rationale: triggers all over. Probably wrong code in some
cases, but careful thought needed in most Bigger project.
sign-conversion (4.6)
Triggers ALL OVER. Quite possibly a bug in some cases, though.
Bigger project.
cast-align (3)
We already do this safely. Need to re-test on a system with
stronger-than-intel alignment rules, though.
shadow (3)
mistake; worth fixing.
switch-default (3)
Not sure this is a good idea; someof these look like mistakes,
but some don't.
assign-enum (clang)
triggers all over; worth fixing.
conditional-uninitialized (clang)
triggers all over; not sure whether this is worth fixing.
```Tor: unspecified