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