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

[Richard Henderson <richard.henderson@linaro.org>]

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.