qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 2/4] hw/ppc/spapr_drc: Replace err


From: Markus Armbruster
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH v2 2/4] hw/ppc/spapr_drc: Replace error_setg(&error_abort) by abort()
Date: Fri, 08 Jun 2018 08:22:59 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)

David Gibson <address@hidden> writes:

> On Fri, Jun 08, 2018 at 12:54:36AM -0300, Philippe Mathieu-Daudé wrote:
>> Hi David,
>> 
>> On 06/08/2018 12:03 AM, David Gibson wrote:
>> > On Thu, Jun 07, 2018 at 11:46:43AM -0300, Philippe Mathieu-Daudé wrote:
>> >> Use abort() instead of error_setg(&error_abort),
>> >> as suggested by the "qapi/error.h" documentation:
>> >>
>> >>     Please don't error_setg(&error_fatal, ...), use error_report() and
>> >>     exit(), because that's more obvious.
>> >>     Likewise, don't error_setg(&error_abort, ...), use assert().
>> >>
>> >> Use abort() instead of the suggested assert() because the assertion is
>> >> already verified by the switch case.
>> > 
>> > I think g_assert_not_reached() would be the right thing here.
>> 
>> I try to follow Eric advice (who recalled Markus).
>> As I understand:
>> http://lists.gnu.org/archive/html/qemu-devel/2017-08/msg03605.html
>> 
>> "glib-Testing [...] should not be used outside of tests/."
>
> Oh, ok, go with that then.

Most g_assert_FOO() are indeed tests-only, but g_assert_not_reached() is
actually acceptable elsewhere, see commit 6e9389563e5, and its review
thread
https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg05499.html
Message-Id: <address@hidden>

That said, I fail to see the value of this kind of lipstick, too.

> Acked-by: David Gibson <address@hidden>
>
>> I can respin if you prefer.

I can replace by g_assert_not_reached() on commit if you prefer.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]