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@xxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
