Re: [PATCH 13/18] display: replace some dubious asserts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]