[Spice-devel] [PATCH v4 04/12] Rephrase assertion section

[Christophe Fergeau]

On Thu, Feb 15, 2018 at 05:06:17PM +0100, Christophe de Dinechin wrote:
> From: Christophe de Dinechin <dinechin at redhat.com>
> 
> Changes since v3:
> - Integrate comments about performance impact and solution
> - Integrate comments about impact of a failed assertion
> - Add prohibition against side effects
> 
> Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
> ---
>  docs/spice_style.txt | 36 +++++++++++++++++++++++++++++++++---
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/spice_style.txt b/docs/spice_style.txt
> index 5dc41cb1..3bc70570 100644
> --- a/docs/spice_style.txt
> +++ b/docs/spice_style.txt
> @@ -113,10 +113,40 @@ FIXME and TODO
>  
>  Comments that are prefixed with `FIXME` describe a bug that need to be fixed. Generally, it is not allowed to commit new code having `FIXME` comment. Committing  `FIXME` is allowed only for existing bugs. Comments that are prefixed with `TODO` describe further features, optimization or code improvements, and they are allowed to be committed along with the relevant code.
>  
> -ASSERT
> -------
> +Assertions
> +----------
> +
> +Use assertions liberally, but pay attention to performance impact. Assertions checks have a significant performance impact should be placed under the `ENABLE_EXTRA_CHECKS` conditional.

[snip]

> +
> +Be mindful of the impact of a failing assertion. For example, assertions in the server should not abort it, as this would kill the QEMU process and the virtual machine.

For me, "assertion" implies abortion of the process, so the first
sentence ("use assertions liberally") and this one are confusing.
In particular, g_assert() which you mention in the snipped part does
abort.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180216/726687d3/attachment.sig>