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

[ Thread Index | Date Index | More Archives ]

Le 11/05/2020 à 11:36, Eero Tamminen a écrit :

On 5/11/20 8:08 AM, Thomas Huth wrote:
Am Sun, 10 May 2020 23:19:24 +0200
schrieb Nicolas Pomarède <npomarede@xxxxxxxxxxxx>:
NOTE : assert() is used in several places ; as the condition/error
tested by assert() can also happen when assert is compiled to no-code
"{}", maybe assert() should be replaced by proper printf+exit (or
return if possible) ?

Only reason to disable asserts should be somebody trying to
make very small Hatari binary for something like Firebee.

There aren't so many of them that they would increase code
size significantly (I hope), and they (or allocs) shouldn't
be in performance critical places.
(Except maybe the one assert in newcpu.c::do_interrupt()
and couple in cycInt.c).

Asserts are there to catch unexpected logic errors.  Although they should be enabled by
default, I think it's OK do disable them in

Actually, I think the best thing that we can do, is to introduce
another wrapper around malloc() in our code base, that prints an error
message and exits the program completely in case of errors, and we then
should use that wrapper for allocating small amounts of memory instead
of using malloc + error handling everywhere around in the code.

The point of asserts is to show places where logic
errors happen. Malloc failure isn't a logic error,
so I think hiding the error location with
a wrapper is fine.

I think logic errors are better handled by a real printf + possible return value to try to continue anyway instead of exiting the program.

If someone is not sure about the logic of his code, assert() can be used, but IMO they should be removed later when logic is fixed. Keeping assert() in years old code in case logic is still wrong looks bad programing to me :) (it's not about the few bytes of code assert adds).

Also, how do we know these assert() won't be removed by some linux distro or windows/mac binaries, depending on global compiler flags we don't control ? And instead of catching a possible error with assert(), the program will just continue and maybe crash later.

That's why I prefer having custom messages in case of unexpected situation that users can report (assert message don't always have all the details to find the cause).

There're approx 40 assert() in hatari code ; for example in video.c :


This might cause issue if this condition is met, but in that case I think the assert should be removed and a proper review of the code's logic should be made to ensure this condition is not possible (in that case, it's not possible). If that assert is met, then memory was certainly corrupted at other place in case "OVERSCAN_TOP >= MAX_OVERSCAN_BOTTOM".


Mail converted by MHonArc 2.6.19+