Hi Jan,
> On 16 Feb 2022, at 09:31, Jan Beulich <jbeul...@suse.com> wrote:
>
> On 16.02.2022 10:25, Bertrand Marquis wrote:
>> Hi Jan, Julien,
>>
>>> On 15 Feb 2022, at 21:00, Julien Grall <jul...@xen.org> wrote:
>>>
>>> (+ Bertrand)
>>>
>>> Hi Jan,
>>>
>>> On 27/01/2022 14:34, Jan Beulich wrote:
>>>> The increasing amount of constructs along the lines of
>>>> if ( !condition )
>>>> {
>>>> ASSERT_UNREACHABLE();
>>>> return;
>>>> }
>>>> is not only longer than necessary, but also doesn't produce incident
>>>> specific console output (except for file name and line number).
>>>
>>> So I agree that this construct will always result to a minimum 5 lines.
>>> Which is not nice. But the proposed change is...
>>>
>>>> Allow
>>>> the intended effect to be achieved with ASSERT(), by giving it a second
>>>> parameter allowing specification of the action to take in release builds
>>>> in case an assertion would have triggered in a debug one. The example
>>>> above then becomes
>>>> ASSERT(condition, return);
>>>> Make sure the condition will continue to not get evaluated when just a
>>>> single argument gets passed to ASSERT().
>>>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>>>> ---
>>>> v2: Rename new macro parameter.
>>>> ---
>>>> RFC: The use of a control flow construct as a macro argument may be
>>>> controversial.
>>>
>>> indeed controversial. I find this quite confusing and not something I would
>>> request a user to switch to if they use the longer version.
>>>
>>> That said, this is mainly a matter of taste. So I am interested to hear
>>> others view.
>>>
>>> I have also CCed Bertrand to have an opinions from the Fusa Group (I
>>> suspect this will go backward for them).
>>
>> Thanks and here is my feedback in regards to Fusa here.
>>
>> Most certification standards are forbidding completely macros including
>> conditions (and quite a number are forbidding static inline with conditions).
>> The main reason for that is MCDC coverage (condition/decisions and not only
>> code line) is not possible to do anymore down to the source code and has to
>> be
>> done down to the pre-processed code.
>>
>> Out of Fusa considerations, one thing I do not like in this solution is the
>> fact that
>> you put some code as parameter of the macro (the return).
>>
>> To make this a bit better you could put the return code as parameter
>> instead of having “return CODE” as parameter.
>
> Except that it's not always "return" what you want, and hence it
> can't be made implicit.
Then having code as parameter for a macro is really not nice I think.
>
>> An other thing is that Xen ASSERT after this change will be quite different
>> from
>> any ASSERT found in other projects which could make it misleading for
>> developers.
>> Maybe we could introduce an ASSERT_RETURN macros instead of modifying the
>> behaviour of the standard ASSERT ?
>
> Along the lines of the above, this would then mean a multitude of
> new macros.
Understood then my statement of Xen having an ASSERT different from any other
known
assert still stands, this is no something I would vote for.
If you want to keep the code then maybe using ASSERT_ACTION or something like
that
and keep ASSERT being a standard assert would be a good idea.
Regards
Bertrand
>
> Jan
