[v1] Use g_assert_not_reached instead of (g_)assert(0, false)

[PATCH 19/39] hw/pci: replace assert(false) with g_assert_not_reached()

Posted by Pierrick Bouvier 2 months, 2 weeks ago

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 hw/pci/pci-stub.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
index f0508682d2b..c6950e21bd4 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -46,13 +46,13 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
 /* kvm-all wants this */
 MSIMessage pci_get_msi_message(PCIDevice *dev, int vector)
 {
-    g_assert(false);
+    g_assert_not_reached();
     return (MSIMessage){};
 }
 
 uint16_t pci_requester_id(PCIDevice *dev)
 {
-    g_assert(false);
+    g_assert_not_reached();
     return 0;
 }
 
-- 
2.39.2

Re: [PATCH 19/39] hw/pci: replace assert(false) with g_assert_not_reached()

Posted by Philippe Mathieu-Daudé 2 months, 2 weeks ago

Hi Pierrick,

On 11/9/24 00:15, Pierrick Bouvier wrote:
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   hw/pci/pci-stub.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
> index f0508682d2b..c6950e21bd4 100644
> --- a/hw/pci/pci-stub.c
> +++ b/hw/pci/pci-stub.c
> @@ -46,13 +46,13 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
>   /* kvm-all wants this */
>   MSIMessage pci_get_msi_message(PCIDevice *dev, int vector)
>   {
> -    g_assert(false);
> +    g_assert_not_reached();
>       return (MSIMessage){};

The tail of this series remove the unreachable 'break' lines.
Why 'return' lines aren't problematic? Is that a GCC TSan bug?

>   }
>   
>   uint16_t pci_requester_id(PCIDevice *dev)
>   {
> -    g_assert(false);
> +    g_assert_not_reached();
>       return 0;
>   }
>

Re: [PATCH 19/39] hw/pci: replace assert(false) with g_assert_not_reached()

Posted by Pierrick Bouvier 2 months, 1 week ago

On 9/10/24 22:50, Philippe Mathieu-Daudé wrote:
> Hi Pierrick,
> 
> On 11/9/24 00:15, Pierrick Bouvier wrote:
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    hw/pci/pci-stub.c | 4 ++--
>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
>> index f0508682d2b..c6950e21bd4 100644
>> --- a/hw/pci/pci-stub.c
>> +++ b/hw/pci/pci-stub.c
>> @@ -46,13 +46,13 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
>>    /* kvm-all wants this */
>>    MSIMessage pci_get_msi_message(PCIDevice *dev, int vector)
>>    {
>> -    g_assert(false);
>> +    g_assert_not_reached();
>>        return (MSIMessage){};
> 
> The tail of this series remove the unreachable 'break' lines.
> Why 'return' lines aren't problematic? Is that a GCC TSan bug?
> 

It's related to how control flow analysis works, but I don't have a 
deeper answer. I reported the issue with 'break' for gcc and the same 
bug was created several years ago, so it was just marked as duplicate.

I'll clean the extra return with have though, as part of v2.

>>    }
>>    
>>    uint16_t pci_requester_id(PCIDevice *dev)
>>    {
>> -    g_assert(false);
>> +    g_assert_not_reached();
>>        return 0;
>>    }
>>    
>

Re: [PATCH 19/39] hw/pci: replace assert(false) with g_assert_not_reached()

Posted by Richard Henderson 2 months, 2 weeks ago

On 9/10/24 15:15, Pierrick Bouvier wrote:
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   hw/pci/pci-stub.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
> index f0508682d2b..c6950e21bd4 100644
> --- a/hw/pci/pci-stub.c
> +++ b/hw/pci/pci-stub.c
> @@ -46,13 +46,13 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
>   /* kvm-all wants this */
>   MSIMessage pci_get_msi_message(PCIDevice *dev, int vector)
>   {
> -    g_assert(false);
> +    g_assert_not_reached();
>       return (MSIMessage){};
>   }
>   
>   uint16_t pci_requester_id(PCIDevice *dev)
>   {
> -    g_assert(false);
> +    g_assert_not_reached();
>       return 0;
>   }
>   

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~