[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Use of g_return_if_fail(), g_return_val_if_fail()
From: |
Marc-André Lureau |
Subject: |
Re: Use of g_return_if_fail(), g_return_val_if_fail() |
Date: |
Tue, 17 Nov 2020 19:34:55 +0400 |
Hi
On Tue, Nov 17, 2020 at 7:14 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> g_return_if_fail(), g_return_val_if_fail() are for programming errors:
>
> If expr evaluates to FALSE, the current function should be
> considered to have undefined behaviour (a programmer error). The
> only correct solution to such an error is to change the module that
> is calling the current function, so that it avoids this incorrect
> call.
>
> Unlike assert(), they continue regardless, undefined behavior be damned:
>
> To make this undefined behaviour visible, if expr evaluates to
> FALSE, the result is usually that a critical message is logged and
> the current function returns.
>
> Except when you ask for abort():
>
> To debug failure of a g_return_if_fail() check, run the code under a
> debugger with G_DEBUG=fatal-criticals or G_DEBUG=fatal-warnings
> defined in the environment.
>
> Like assert(), they can be compiled out:
>
> If G_DISABLE_CHECKS is defined then the check is not performed. You
> should therefore not depend on any side effects of expr .
>
> There are just three uses outside contrib/:
>
> * backends/dbus-vmstate.c:232: g_return_val_if_fail(bytes_read == len,
> -1);
>
> Marc-André, why is bytes_read != len a programming error?
>
> Why is returning safe?
It's "safe" as it returns -1 to indicate an error to post_load callback.
Hmm, it may not be just a programming error. read_all() may return
success with less bytes than requested.
Here, replacing it with full error_report() may be more appropriate,
since possibly the condition could happen if the input stream is
malformed. I can send a patch.
g_return* would be fine if it was just a programming error (checking
read_all contract for example).