Hi Heather,
On Tue, 2023-10-10 at 15:42 +0200, Mark Wielaard wrote:
> From: Heather McIntyre <h...@rice.edu>
>
> * elf_version.c (version_once): Define once.
> (initialize_version): New static function.
> (elf_version): Use initialize_version version_once.
>
> Signed-off-by: Heather S. McIntyre <h...@rice.edu>
> Signed-off-by: Mark Wielaard <m...@klomp.org>
> ---
> libelf/elf_version.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/libelf/elf_version.c b/libelf/elf_version.c
> index 6ec534ab..8296bb65 100644
> --- a/libelf/elf_version.c
> +++ b/libelf/elf_version.c
> @@ -32,12 +32,21 @@
> #endif
>
> #include <libelfP.h>
> +#include <pthread.h>
>
> +/* Multiple threads may initialize __libelf_version.
> + pthread_once() ensures that __libelf_version is initialized only once. */
> +once_define(static, version_once);
>
> /* Currently selected version. Should be EV_CURRENT.
> Will be EV_NONE if elf_version () has not been called yet. */
> unsigned int __libelf_version = EV_NONE;
>
> +static void initialize_version(void)
> +{
> + __libelf_version = EV_CURRENT;
> +}
> +
> unsigned int
> elf_version (unsigned int version)
> {
> @@ -49,7 +58,7 @@ elf_version (unsigned int version)
> /* Phew, we know this version. */
>
> /* Signal that the version is now initialized. */
> - __libelf_version = EV_CURRENT;
> + once(version_once, initialize_version);
>
> /* And return the last (or initial) version. */
> return EV_CURRENT;
This is an odd function. The intention clearly was to support more "ELF
versions" at some point. But (luckily) that never happened and I doubt
there will ever be a different (incompatible) ELF version that we'll
have to support. So in the end this will always be EV_CURRENT == 1. But
the function has to be called to make the rest of the library work.
I think this works and is fine. There will most likely never be real
contention calling elf_version because normally a program just calls it
once at the start.
But have you thought about using some atomic operation here instead?
Cheers,
Mark
