Hi,
On 5/11/20 12:53 PM, Nicolas Pomarède wrote:
Le 11/05/2020 à 11:36, Eero Tamminen a écrit :
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.
By logic error I mean "this code didn't work as
it should, all bets are now off". You shouldn't
continue at that point, after that point things
will get only progressively harder to debug.
If assert is on a condition that can happen and
from which you can recover, you shouldn't use an
assert.
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).
They are there to guard against new changes
accidentally breaking old code. Years later you
might not anymore remember exactly how it was
supposed to work. For example profiling call
graph collection is of that complexity.
Assert could in theory be triggered also by
something (e.g. array overwrite) corrupting
Hatari's internal state.
You could think of an assert as a unit test.
You don't remove unit tests after they stop
triggering.
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.
Asserts shouldn't be used for a potential errors,
but for "can't happen" things. While they should
be enabled for all development builds, disabling
them for releases could be OK as code changes in
those should have already been tested.
In releases they might still catch memory corruption issues though, e.g.
due to different
build environments, so it would still be
preferable for them to be enabled in releases.