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

[Thomas Huth <thuth@redhat.com>]

On 22/02/2023 00.25, Philippe Mathieu-Daudé wrote:
> In order to avoid warnings such commit c0a6665c3c ("target/i386:
> Remove compilation errors when -Werror=maybe-uninitialized"),
> replace all assert(0) and g_assert(0) by g_assert_not_reached().
> 
> Remove any code following g_assert_not_reached().
> 
> See previous commit for rationale.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> 
> diff --git a/docs/spin/aio_notify_accept.promela b/docs/spin/aio_notify_accept.promela
> index 9cef2c955d..f929d30328 100644
> --- a/docs/spin/aio_notify_accept.promela
> +++ b/docs/spin/aio_notify_accept.promela
> @@ -118,7 +118,7 @@ accept_if_req_not_eventually_false:
>       if
>           :: req -> goto accept_if_req_not_eventually_false;
>       fi;
> -    assert(0);
> +    g_assert_not_reached();
>   }

This does not look like C code ... is it safe to replace the statement here?

> diff --git a/docs/spin/aio_notify_bug.promela b/docs/spin/aio_notify_bug.promela
> index b3bfca1ca4..ce6f5177ed 100644
> --- a/docs/spin/aio_notify_bug.promela
> +++ b/docs/spin/aio_notify_bug.promela
> @@ -106,7 +106,7 @@ accept_if_req_not_eventually_false:
>       if
>           :: req -> goto accept_if_req_not_eventually_false;
>       fi;
> -    assert(0);
> +    g_assert_not_reached();
>   }

dito

> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index f54f44d899..59c8032a21 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1347,49 +1347,42 @@ int postcopy_ram_incoming_init(MigrationIncomingState *mis)
>   
>   int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
>   {
> -    assert(0);
> -    return -1;
> +    g_assert_not_reached();
>   }
>   
>   int postcopy_ram_prepare_discard(MigrationIncomingState *mis)
>   {
> -    assert(0);
> -    return -1;
> +    g_assert_not_reached();
>   }
>   
>   int postcopy_request_shared_page(struct PostCopyFD *pcfd, RAMBlock *rb,
>                                    uint64_t client_addr, uint64_t rb_offset)
>   {
> -    assert(0);
> -    return -1;
> +    g_assert_not_reached();
>   }
>   
>   int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
>   {
> -    assert(0);
> -    return -1;
> +    g_assert_not_reached();
>   }
>   
>   int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
>                           RAMBlock *rb)
>   {
> -    assert(0);
> -    return -1;
> +    g_assert_not_reached();
>   }
>   
>   int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
>                           RAMBlock *rb)
>   {
> -    assert(0);
> -    return -1;
> +    g_assert_not_reached();
>   }
>   
>   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, 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?

  Thomas