Re: [Xen-devel] [PATCH 01/10] x86: assembly, ENTRY for fn, GLOBAL for data

[Ingo Molnar <mingo@xxxxxxxxxx>]

* hpa@xxxxxxxxx <hpa@xxxxxxxxx> wrote:

> On March 1, 2017 2:27:54 AM PST, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >
> >* Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:
> >
> >> On Wed, 1 Mar 2017, Ingo Molnar wrote:
> >> > 
> >> > * Jiri Slaby <jslaby@xxxxxxx> wrote:
> >> > 
> >> > > This is a start of series to unify use of ENTRY, ENDPROC, GLOBAL,
> >END,
> >> > > and other macros across x86. When we have all this sorted out,
> >this will
> >> > > help to inject DWARF unwinding info by objtool later.
> >> > > 
> >> > > So, let us use the macros this way:
> >> > > * ENTRY -- start of a global function
> >> > > * ENDPROC -- end of a local/global function
> >> > > * GLOBAL -- start of a globally visible data symbol
> >> > > * END -- end of local/global data symbol
> >> > 
> >> > So how about using macro names that actually show the purpose,
> >instead of 
> >> > importing all the crappy, historic, essentially randomly chosen
> >debug symbol macro 
> >> > names from the binutils and older kernels?
> >> > 
> >> > Something sane, like:
> >> > 
> >> >  SYM__FUNCTION_START
> >> 
> >> Sane would be:
> >> 
> >>            SYM_FUNCTION_START
> >> 
> >> The double underscore is just not giving any value.
> >
> >So the double underscore (at least in my view) has two advantages:
> >
> >1) it helps separate the prefix from the postfix.
> >
> >I.e. it's a 'symbols' namespace, and a 'function start', not the
> >'start' of a 
> >'symbol function'.
> >
> >2) It also helps easy greppability.
> >
> >Try this in latest -tip:
> >
> >  git grep e820__
> >
> >To see all the E820 API calls - with no false positives!
> >
> >'git grep e820_' on the other hand is a lot less reliable...
> >
> >But no strong feelings either way, I just try to sneak in these small
> >namespace 
> >structure tricks when nobody's looking! ;-)
> >
> >Thanks,
> >
> >     Ingo
> 
> This seems needlessly verbose to me and clutters the code.
> 
> How about:
> 
> PROC..ENDPROC, LOCALPROC..ENDPROC and DATA..ENDDATA.  Clear, unambiguous and 
> balanced.

I'm sorry, but that naming scheme is disgusing, it reminds me of BASIC 
nomenclature ... did we run out of underscores or what?

Nor would clearly structured names clutter anything, this isn't going to be 
used 
deep inside nested code, it's going to be the single level syntactic term in 
addition to the symbol name itself:

        SYM__FUNCTION_START(some_kernel_asm_function)
        ...
        SYM__FUNCTION_END(some_kernel_asm_function)

We could shorten it to 'FUNC' if length is really an issue:

        SYM__FUNC_START(some_kernel_asm_function)
        ...
        SYM__FUNC_END(some_kernel_asm_function)

Also, 'PROC', presumably standing for 'procedure', is both ambiguous and a 
misnomer:

 - it's ambiguous with commonly used abbreviations of procfs and process

 - C functions are not actually 'procedures'. Procedures in PASCAL style 
languages
   denote functions that don't return any values. Most of the kernel asm 
functions
   actually do. I realize that even in C we sometimes talk about 'procedures' 
out
   of hysterical inertia, but if you check the C standards, most of them don't
   even use the term 'procedure'.

'function' on the other hand is totally unambiguous.

Plus the lack of START/STOP (or BEGIN/END) makes it easy to commit the mistake 
of 
forgetting to add the end marker, and your naming scheme preserves that!

So if we fix/extend these macros then _PLEASE_ we need to get this stupid, 
illogical, nonsensical, external tooling inherited naming craziness fixed 
first, 
not doubled down on...

</rant>

Thanks,

        Ingo

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel