Re: Use of g_return_if_fail(), g_return_val_if_fail()

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

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).