Re: [PATCH 3/5] KVM: arm64: Rename __vcpu_sys_reg -> vcpu_sys_reg

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Marc,

On 7/14/21 11:23 AM, Marc Zyngier wrote:
> Hi Alex,
>
> On Wed, 14 Jul 2021 10:55:59 +0100,
> Alexandru Elisei <alexandru.elisei@arm.com> wrote:
>> There are two macros to access a specific system register from a known
>> kvm_cpu_context: __ctxt_sys_reg(), which returns a pointer to the register,
>> and ctxt_sys_reg(), which deferences the pointer returned by
>> __ctxt_sys_reg().
>>
>> __vcpu_sys_reg() serves a similar purpose, with the difference being that
>> it takes a struct kvm_vcpu as a parameter. __vcpu_sys_reg(), although it
>> looks like __ctxt_sys_reg(), it dereferences the pointer to the register,
>> like ctxt_sys_reg() does, and indeed it is defined as an abstraction over
>> ctxt_sys_reg().
>>
>> Let's remove this naming inconsistency by renaming __vcpu_sys_reg() to
>> vcpu_sys_reg(), to make it clear it behaves like ctxt_sys_reg(), and not
>> like __ctxt_sys_reg().
> I can't say I'm keen on this change.
>
> The leading underscores really are there to outline that *this is
> dangerous*, as you really need to know which context you are in.
> Dropping the leading '__' may give the false impression that this is
> safe, and not actually a primitive that requires careful thinking
> before use.
>
> ctxt_sys_reg() is, on the other hand, clearly something that acts
> solely on memory because it takes a context structure, and not a
> vcpu. At least that's what the 'ctxt' prefix is supposed to convey
> (not very successfully, apparently).

Oh, so that's the real reason for the leading underscores, I assumed that the
comment was warning enough. Since the 3 macros were right next to each other, and
__vcpu_sys_reg() is a wrapper over ctxt_sys_reg(), I thought they're there to
differentiate between returning a reference and returning a value. I'll drop this
patch and the one after it.

Thanks,

Alex

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm