Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum

[Marc-André Lureau]

Hi

On Fri, Jun 2, 2017 at 10:30 AM Markus Armbruster <address@hidden> wrote:

> Marc-André Lureau <address@hidden> writes:
>
> > Hi
> >
> > On Tue, May 30, 2017 at 6:23 PM Markus Armbruster <address@hidden>
> wrote:
> >
> >> Marc-André Lureau <address@hidden> writes:
> >>
> >> > Hi
> >> >
> >> > On Thu, May 11, 2017 at 6:30 PM Markus Armbruster <address@hidden>
> wrote:
> [...]
> >> >> g_assert_not_reached() is problematic, see "[PATCH] checkpatch:
> Disallow
> >> >> glib asserts in main code".
> >> >>
> >> >> Message-Id: <address@hidden>
> >> >> https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg05499.html
> >> >>
> >> >>
> >> > Actually g_assert() and g_assert_not_reached() are accepted.
> >>
> >> What exactly does g_assert() buy us over plain assert(), and
> >> g_assert_not_reached() over assert(0)?
> >>
> >
> > g_assert() brings a bit more context, afaik, can be trapped for error
> > testing, and error reporting can be handled by an handler. Not that
> useful
> > to qemu, but could be for the graphical UI though.
> >
> > g_assert_not_reached() is quite more readable than assert(0)
>
> I'm all for making intent explicit, but what else could assert(0)
> possibly mean?
>
> >> qapi/ overwhelmingly uses assert().
> >>
> >
> > ok, it's already a mix of assert & g_assert in qemu though
>
> True.
>
> In my opinion, we should use only one outside tests.  g_assert() if it
> adds value, else plain assert().  "Outside tests", because g_assert()
> might add sufficient value in tests even when it doesn't elsewhere.
>
>
g_assert*() is useful in the unit under test, so you can check the assert
behaviour. This could be useful in particular for the assertions under
qapi/ since we have unit tests for those


> Until then, I prefer to use only one *locally*.  In qapi/, that's plain
> assert() now.
>

ok, fair enough
-- 
Marc-André Lureau