On 27.10.20 19:53, Andrea Bastoni wrote:
> On 27/10/2020 14:10, Jan Kiszka wrote:
>> On 22.10.20 19:58, Andrea Bastoni wrote:
>>> Substitute the implicit "-1" occurrence for an invalid CPU id with an
>>> unsigned int INVALID_CPU_ID that can be used in all "uint-related"
>>> comparisons.
>>>
>>> Signed-off-by: Andrea Bastoni <[email protected]>
>>> ---
>>>  hypervisor/arch/arm-common/lib.c       | 2 +-
>>>  hypervisor/arch/arm-common/psci.c      | 4 ++--
>>>  hypervisor/control.c                   | 2 +-
>>>  hypervisor/include/jailhouse/control.h | 5 +++--
>>>  hypervisor/setup.c                     | 4 ++--
>>>  5 files changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hypervisor/arch/arm-common/lib.c 
>>> b/hypervisor/arch/arm-common/lib.c
>>> index 916cd54f..889b3d14 100644
>>> --- a/hypervisor/arch/arm-common/lib.c
>>> +++ b/hypervisor/arch/arm-common/lib.c
>>> @@ -31,5 +31,5 @@ unsigned int arm_cpu_by_mpidr(struct cell *cell, unsigned 
>>> long mpidr)
>>>             if (mpidr == (public_per_cpu(cpu)->mpidr & MPIDR_CPUID_MASK))
>>>                     return cpu;
>>>  
>>> -   return -1;
>>> +   return INVALID_CPU_ID;
>>>  }
>>> diff --git a/hypervisor/arch/arm-common/psci.c 
>>> b/hypervisor/arch/arm-common/psci.c
>>> index 6a9abf60..242cad5b 100644
>>> --- a/hypervisor/arch/arm-common/psci.c
>>> +++ b/hypervisor/arch/arm-common/psci.c
>>> @@ -27,7 +27,7 @@ static long psci_emulate_cpu_on(struct trap_context *ctx)
>>>     long result;
>>>  
>>>     cpu = arm_cpu_by_mpidr(this_cell(), ctx->regs[1] & mask);
>>> -   if (cpu == -1)
>>> +   if (cpu == INVALID_CPU_ID)
>>>             /* Virtual id not in set */
>>>             return PSCI_DENIED;
>>>  
>>> @@ -63,7 +63,7 @@ static long psci_emulate_affinity_info(struct 
>>> trap_context *ctx)
>>>  {
>>>     unsigned int cpu = arm_cpu_by_mpidr(this_cell(), ctx->regs[1]);
>>>  
>>> -   if (cpu == -1)
>>> +   if (cpu == INVALID_CPU_ID)
>>>             /* Virtual id not in set */
>>>             return PSCI_DENIED;
>>>  
>>> diff --git a/hypervisor/control.c b/hypervisor/control.c
>>> index b38ac2e9..0078ef19 100644
>>> --- a/hypervisor/control.c
>>> +++ b/hypervisor/control.c
>>> @@ -48,7 +48,7 @@ unsigned long panic_cpu = -1;
>>>   * @note For internal use only. Use for_each_cpu() or for_each_cpu_except()
>>>   * instead.
>>>   */
>>> -unsigned int next_cpu(unsigned int cpu, struct cpu_set *cpu_set, int 
>>> exception)
>>> +unsigned int next_cpu(unsigned int cpu, struct cpu_set *cpu_set, unsigned 
>>> int exception)
>>
>> Overlong line.
> 
> OK. Didn't think this rule was too strict when the readability was slightly
> better and since I find other places where it was not enforced.
> 

There are exception (long error messages) and there might be a few
missed cases.

> But readability is a subjective matter anyway...
> 
> In addition to my patches, should I fix the other occurrences or do you prefer
> to only change those if that code changes for other reasons?
> 

Primarily avoid adding new violations. We do not need to run actively
after such cases, though, I guess there is enough other stuff :)

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jailhouse-dev/8ad705e0-fec5-7b97-9fe2-d949b1fec58f%40siemens.com.

Reply via email to