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

[Thomas Huth]

On 22/02/2023 07.29, Richard Henderson wrote:
> 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().

OK, fair, makes sense. I now slightly remember we also had similar 
discussions in the past already, so these hunks now sound fine to me, too.

 Thomas