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

[Christophe de Dinechin]

> On 16 Feb 2018, at 10:12, Christophe Fergeau <cfergeau at redhat.com> wrote:
> 
> 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,

Which is why I point out the impact.

> 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.

So? I don’t understand the problem. How would you rephrase?

> 
> Christophe