[Spice-devel] [PATCH 13/18] display: replace some dubious asserts

[Uri Lublin]

On 11/27/2015 11:54 AM, Frediano Ziglio wrote:
>>
> What about:
>
>
> display: move checks inside display_channel_wait_for_migrate_data
>
> Instead of relaying on the caller to use red_channel_waits_for_migrate_data
> to check if there are client waiting for migration data check inside the
> function. This make sure that the check is done so some check can be removed
> from the function.
>
>
> Frediano
>

Hi Frediano,

In this specific case the asserts' conditions were already tested by the
(only) calling function. If someone decides to call this function
without verifying those conditions that would be an error we want
to discover early. Them being there also act as a hint for the reader
that those conditions are assumed to be true (although that's
also true for spice_return_if lines).

Your solution works, as it moves the responsibility of the checks
within the function. Although you may still be inclined to verify
(assert) that those conditions are true before proceeding.

Personally I do not mind having those asserts in the code.
If people prefer, we can also return_if_fail and let the
calling functions deal with failures (which it currently
does not, as it checks those conditions itself).
So in the patch we can replace spice_warn_if_fail with
spice_return_val_if_fail.
Also add a commit message saying that those checks
are being tested by the calling function and that the calling
function needs to handle failures.

Regards,
     Uri.