On 26/04/17 07:04, Jan Beulich wrote:
>>>> On 25.04.17 at 16:52, <andrew.coop...@citrix.com> 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@lists.xen.org
https://lists.xen.org/xen-devel
