This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PING: PATCH: Automatically test IFUNC implementations


> +	* Rules (tests): Filter out $(tests-ifunc) if multi-arch isn't
> +	enabled.
> +	(xtests): Filter out $(xtests-ifunc) if multi-arch isn't enabled.
> +	* include/libc-func.h: New file.
> +	* misc/libc-func.c: Likewise.
> +	* misc/Makefile (routines): Add libc-func if multi-arch is
> +	enabled.

Be technically precise about conditionals:

	* Rules [$(multi-arch) = no] (tests): Filter out $(tests-ifunc).

etc.

> +	* misc/Versions: Add __libc_supported_implementations to
> +	GLIBC_PRIVATE

	* misc/Versions (GLIBC_PRIVATE): Add __libc_supported_implementations.

> +	* string/test-string.h: Include <libc-func.h>.
> +	(func_list): New.  Defined only if TEST_IFUNC and TEST_NAME are
> +	defined.
> +	(func_count): Likewise.
> +	(impl_count): Likewise.
> +	(impl_array): Likewise.

New what?!?!  And we have a convention for conditionals.  Use it!

	[TEST_IFUNC && TEST_NAME] (func_list, func_count): New variables.

etc.

> +++ b/include/libc-func.h

This is a pretty poor name given the function's renaming.
Since this purely for IFUNC cases, lets have ifunc in the name.
How about libc-ifunc-impl-list.h, struct libc_ifunc_impl, and
__libc_ifunc_impl_list (ifunc-impl-list.c)?

> +ifneq ($(multi-arch),no)
> +routines   += libc-func
> +endif

Let's just make it unconditional and let the trivial stub version be built
when multiarch is not enabled.

> +/* Default __libc_supported_implementations.

/* Enumerate available IFUNC implementations of a function.  Stub version.

> +/* Fill ARRAY of MAX elements with IFUNC implementations for function
> +   NAME supported on target machine and return the number of valid

"on this machine"

This whole scheme doesn't include any way to notice which or how many
implementations you aren't testing because your machine is incompatbile.

I'm a bit inclined to make the struct include a "bool usable" field, have
the array always filled with all function pointers, and let the testers
skip the ones with usable==false.  Then the testers can emit output saying
"there were N variants of foo I could not test".  Eventually we can feed
this into some more full scheme to ensure that all the variants have really
been tested well.
> +	  for (impl = __start_impls; impl < __stop_impls; ++impl)	\
> +	    impl_count++;						\

A loop is a lousy way to calculate a subtraction.

> +#if defined TEST_IFUNC && defined TEST_NAME
> +# define SIZEOF_ARRAY(array) (sizeof array / sizeof array[0])
> +  func_count = __libc_supported_implementations (TEST_NAME, func_list,
> +						 SIZEOF_ARRAY (func_list));

Don't define a macro to use it only once.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]