Re: [PATCH 3/5] bulk: Replace [g_]assert(0) -> g_assert_not_reached()

[Richard Henderson]

On 2/21/23 18:06, Thomas Huth wrote:
> >   int postcopy_wake_shared(struct PostCopyFD *pcfd,
> >                            uint64_t client_addr,
> >                            RAMBlock *rb)
> >   {
> > -    assert(0);
> > -    return -1;
> > +    g_assert_not_reached();
> >   }
> >   #endif
>
> If we ever reconsider to allow compiling with G_DISABLE_ASSERT again,

... and we shouldn't [1] ...

> this will fail to compile since the return is missing now, so this is kind of 
> ugly ... would it make sense to replace this with g_assert_true(0) instead? Or 
> use abort() directly?

With g_assert_true(0), definitely not.
That is a testing-only item which can be disabled at runtime.

With abort(), no, since g_assert_not_reached() prints file:line.
Indeed, I was suggesting the opposite -- to replace abort() without error_report() with 
g_assert_not_reached().


r~


[1] Allowing G_DISABLE_ASSERT and/or NDEBUG would only require that we invent 
qemu-specific replacements with either (1) do exactly the same thing or, (2) interact with 
__builtin_unreachable() or __builtin_trap(), so that we tell the compiler exactly what's 
going on with the expressions and flow control.