Re: [Qemu-ppc] [PATCH v3 1/8] target-ppc: Add helper for KVM_PPC_RTAS_DEFINE_TOKEN

[Alexander Graf]

On 31.08.2013, at 00:19, Benjamin Herrenschmidt wrote:

> On Fri, 2013-08-30 at 16:34 +0200, Alexander Graf wrote:
> 
>>> So - kvmppc_define_rtas_in_kernel() or kvmppc_define_rtas_kernel_token()?
>> 
>> The former probably.
> 
> I told him the latter :-) Honestly, your name is gross :-) Mine
> describes exactly what the calls does.

I really don't care which one, as long as it describes what it does. Both names 
fit that description, just that the former is smaller :). Really, pick any, as 
long as a random reader has the chance to know what's going on.

> 
>>> I would actually though that the very first "k" is for "Kernel" already 
>>> but...
>> 
>> I a symmetric API you could also remove rtas handling from the kernel, so 
>> you could tell the kernel "handle this in kernel space" and "don't handle 
>> this in kernel space". Our naming needs to be precise enough that you can 
>> guess what is going on from it.
> 
> But that's not quite what the API is doing and we don't want a symetric
> API it's pointless.
> 
> Yet another "my CS teacher told me a good API is symetric" (oh and
> that's a nice word "symetric", it sounds *good*, using it makes it
> look like we are plenty smart !) but in the end totally pointless
> for the problem at hand.

Imagine you don't know the KVM API in this case, but simply read the kvm code. 
You don't know whether it's symmetric or not. It's not, and I don't think it's 
a big issue that it isn't. But the reader doesn't know at this point.

So now you need to figure out what's going on. Either the function name tells 
you (no time loss), you know where to find the KVM API documentation and look 
it up there (small time loss) or you check the actual code (even more time 
loss).

Overall, I'm fairly sure we wasted more time arguing on this one by now than 
anyone would ever waste on looking up anything on this function.


Alex