Re: [PATCH] x86/HVM: fold hypercall tables

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/02/16 08:52, Jan Beulich wrote:
>>>> On 15.02.16 at 09:26, <andrew.cooper3@citrix.com> wrote:
>> On 15/02/2016 07:42, Jan Beulich wrote:
>>> @@ -5395,7 +5398,7 @@ int hvm_do_hypercall(struct cpu_user_reg
>>>          }
>>>  #endif
>>>  
>>> -        regs->_eax = hvm_hypercall32_table[eax](ebx, ecx, edx, esi, edi, ebp);
>>> +        regs->_eax = hypercall_table[eax].compat(ebx, ecx, edx, esi, edi, 
>> ebp);
>>>  
>> I know its in a different translation unit, but we already have a
>> hypercall_table and it is a global symbol.  Please could we retain the
>> hvm_ prefix here.
> I'm aware of that and removed the prefix knowingly. I see no
> reason why it needs to be there.

Redundant prefixes like this are not for the benefit of the compiler. 
They are for the benefit of humans reading the code.

You, as an individual who is very familiar with the codebase, might have
no problem identifying which actual symbol is intended.

Being open source, the intended audience is the average C programmer who
can make alterations, but isn't completely familiar with the codebase,
and likely be navigating the source code with tools such as
grep/cscope/ctags/other.

Whether the symbols are static or not, they should distinguishable to
humans.

>  With static symbols now getting
> prefixed by their file names in stack dumps and alike, I think we
> should actually get rid of any such redundant static symbol name
> prefixes (scheduler code being one of the biggest "user" of such).

The problem with the scheduler code is that it used to use the same
static prefix for different schedulers.

>
>> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Please clarify whether the R-b stands nevertheless, or whether
> we need to have a longer debate first.

My R-b does not stand.

~Andrew