Re: [Xen-devel] [PATCH for-next v2 09/10] x86/domain: move PV specific code to pv/domain.c

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/04/17 07:04, Jan Beulich wrote:
>>>> On 25.04.17 at 16:52, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 25/04/17 14:52, Wei Liu wrote:
>>> +
>>> +    for_each_vcpu( d, v )
>>> +    {
>>> +        rc = setup_compat_arg_xlat(v);
>>> +        if ( !rc )
>>> +            rc = setup_compat_l4(v);
>>> +
>>> +        if ( rc )
>>> +            goto undo_and_fail;
>> This is odd structuring.  Care to rearrange it with two goto's, rather
>> than inverting the first rc check?
> I don't see anything odd about it - just like with preferably limiting
> the number of return statements, I think limiting the number of
> goto-s is quite desirable. What if the second if()'s body had more
> than just a goto? I'd certainly prefer the code structure above in
> that case over _adding_ a goto into this second if(). Further down
> the same two functions are being called, pointlessly using two
> goto-s. If you really wanted to get rid of the inverted first
> condition, then how about if ( (rc = ...) || (rc = ...) ) ?

For functions which have standard rc semantics, you can actually chain
them together like:

rc = setup_compat_arg_xlat(v) ?: setup_compat_l4(v) ?: ... ;

which will break at the first non-zero return in the chain, and allows
you to have a single assignment and single goto.

Whether this style is sensible to follow is a different matter, but it
certainly is compact.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel