Re: [hatari-devel] fixing errors reported by GCC 10 -fanalyzer

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Tue, 12 May 2020 22:50:27 +0200
schrieb Nicolas Pomarède <npomarede@xxxxxxxxxxxx>:

> Le 11/05/2020 à 17:05, Eero Tamminen a écrit :
> 
> > "Impossible things" shouldn't happen in Hatari's final releases if 
> > they've been properly tested, so I think it's OK to disable
> > asserts for them (although I would prefer them being enabled
> > there also).  
> 
> I agree that in a perfect world that would be the case, but bugs
> reports over the year unfortunately show that users don't use
> devel/alpha/beta version of hatari and that we don't have enough man
> power to go through lots of games/demos testing before each release
> to ensure we don't have regression or bugs or crashes.
> 
> Most of the time, we get valuables/reproducable bug reports once the 
> version is released (without assert() ). Not every bug in that case
> lead to a crash, but I'm afraid the user base of devel version is not
> large enough to ensure "impossible things" have all been tested and
> won't happen on the final release.

But if a user hits a problem with the release build, I think it's only
fair to ask them to try the daily snapshot builds instead - which might
have already the bug fixed, but at least should be compiled with
assert()s enabled.

By the way, Christer, Troed, do you compile your binaries in "Release"
mode or in "Debug" mode?

> My point is more : each time there's a assert(), shouldn't we try to 
> remove it by studying all entry points of the code with that assert
> and ensure the cause is cured at the root ?

We could - but there is still the problem of future regressions: You
add code to a completely different place, and suddenly it triggers an
assert() in another part of the program which you did not expect.
Happens rarely in Hatari, but I've seen it in other projects. So I'd
rather would not remove assert() statements, unless they are really
trivial.

> I mean why "drive" is tested with an assert and not "nHBL" or 
> "cycles_per_line" or any variable that can index an array ?

We should maybe add some assert()s for nHBL and cycles_per_line ...
IIRC, we had some out-of-bounds array accesses related to these
variables in the past...

By the way, some automatic regression tests that excersize the border
removal and spec512 code would be great... does anybody have some code
and/or ideas that could be used for this?

 Thomas