On Fri, Nov 10, 2023 at 11:18:39AM +0000, Peter Maydell wrote:
On Fri, 10 Nov 2023 at 11:02, Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> On Fri, 10 Nov 2023 12:21, Peter Maydell <peter.maydell@linaro.org> wrote:
> >This kind of thing is why Coverity's unused-result warning has a
> >lot of false positives. We shouldn't introduce extra code like
> >this to work around the fact that the tooling doesn't understand
> >our error-handling convention (i.e. error_fatal, and the way
> >that some functions report errors both via the return value and
> >also via the Error** argument).
>
> I respect that :). But I personally believe that clinging to C's
> inadequacies, instead of preventing bugs statically just because it adds
> some lines of code, is misguided. Proper code should strive to make bugs
> impossible in the first place.
I generally agree. The problem here really is that we've ended
up with this odd API convention that reports errors in two
ways. In an ideal world we'd tidy up our APIs to report errors
exactly in one way (presumably via the Error).
The compelling thing about our slightly odd &error_fatal/error_abort
approach is that we can directly get stack traces showing exactly
where the error happened.
If we just propagate the error up the stack to finally exit/abort,
the origin context is essentially lost, and when it does abort, we
don't get a snapshot of all threads at the time the error hit, as
we've moved on in time.