Re: [PATCH] Make __get_cpu_features hidden

[Roland McGrath <roland at hack dot frob dot com>]

> +#define libm_ifunc(name, expr) libc_ifunc (name, (expr))

Extra parens are not actually necessary here.

> +ifeq (yesyes,$(build-shared)$(multi-arch))
> +tests += tst-ifunc-cpu1-main
> +modules-names += tst-ifunc-cpu1-mod
> +
> +tst-ifunc-cpu1-mod.so-no-z-defs = yes

Add a comment about this variable.

> --- /dev/null
> +++ b/sysdeps/x86/tst-ifunc-cpu1-main.c
> @@ -0,0 +1,10 @@
> +/* Test function pointer to local STT_GNU_IFUNC function.  */

Needs a copyright header.

> --- /dev/null
> +++ b/sysdeps/x86/tst-ifunc-cpu1-mod.c

Likewise.

> +void * foo_ifunc (void) __asm__ ("foo") attribute_hidden;
> +__asm__(".type foo, %gnu_indirect_function");

Extra space after * here.  Just use asm, not need for __ification
in test code.

> --- a/sysdeps/x86_64/multiarch/init-arch.c
> +++ b/sysdeps/x86_64/multiarch/init-arch.c
> @@ -19,6 +19,9 @@
> 
>  #include <atomic.h>
>  #include <cpuid.h>
> +/* Define __get_cpu_features.  */
> +#undef __always_inline
> +#define __always_inline
>  #include "init-arch.h"

Defining away a macro like this always needs a comment.  It's better to use
a macro explicitly for this purpose rather than to redefine a standard macro.
That is easy to do since these are only internal headers.

Those nits aside, this needs a fresh posting with a clear explanation of
what problem it's solving and without a ton of old message citations.


Thanks,
Roland