On 15.11.2021 11:13, Bertrand Marquis wrote:
>> On 11 Nov 2021, at 17:57, Andrew Cooper <andrew.coop...@citrix.com> wrote:
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -1234,15 +1234,18 @@ int vcpu_unpause_by_systemcontroller(struct vcpu *v)
>> return 0;
>> }
>>
>> -static void do_domain_pause(struct domain *d,
>> - void (*sleep_fn)(struct vcpu *v))
>> +static void _domain_pause(struct domain *d, bool sync /* or nosync */)
>
> Here you use comments inside the function definition.
> I think this is something that should be avoided and in this specific case a
> boolean sync is clear enough not to need to explain that false is nosing.
While I agree the comment here isn't overly useful, I think ...
>> @@ -1250,12 +1253,12 @@ static void do_domain_pause(struct domain *d,
>> void domain_pause(struct domain *d)
>> {
>> ASSERT(d != current->domain);
>> - do_domain_pause(d, vcpu_sleep_sync);
>> + _domain_pause(d, true /* sync */);
> Same here.
... comments like this one are pretty useful to disambiguate the plain
"true" or "false" (without the reader needing to go look at the function
declaration or definition).
Jan
