Re: [PATCH 0/4] vhost-user: avoid g_return_val_if() in get/set_config()

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Wed, Nov 18, 2020 at 1:17 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
>> Markus Armbruster pointed out that g_return_val_if() is meant for
>> programming
>> errors. It must not be used for input validation since it can be compiled
>> out.
>> Use explicit if statements instead.
>>
>> This patch series converts vhost-user device backends that use
>> g_return_val_if() in get/set_config().
>>
>> Stefan Hajnoczi (4):
>>   contrib/vhost-user-blk: avoid g_return_val_if() input validation
>>   contrib/vhost-user-gpu: avoid g_return_val_if() input validation
>>   contrib/vhost-user-input: avoid g_return_val_if() input validation
>>   block/export: avoid g_return_val_if() input validation
>>
>>
> The condition is the same for all the patches, checking the message config
> payload is large enough. Afaict, the value is set by the client, so it
> could be a runtime error, and thus explicit checking is required.
>
> Nevertheless, one nice thing about g_return* macros, is that it provides an
> error message when the condition fails, which helps. Could you replace it?
>
> (fwiw, I think g_return* macros are so convenient, I would simply make
> G_DISABLE_CHECKS forbidden and call it a day)

Nice or not, they are as inappropriate for input validation as assert()
is:

    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.