Re: [PATCH 15/28] elf: Do not pass GLRO(dl_platform), GLRO(dl_platformlen) to _dl_important_hwcaps

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

LGTM, thanks.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote:
> In the current code, the function can easily obtain the information
> on its own.
> ---
>  elf/dl-hwcaps.c            | 11 +++++------
>  elf/dl-load.c              |  3 +--
>  sysdeps/generic/ldsodefs.h | 12 ++++++------
>  3 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/elf/dl-hwcaps.c b/elf/dl-hwcaps.c
> index 6df9efb255..44dbac099f 100644
> --- a/elf/dl-hwcaps.c
> +++ b/elf/dl-hwcaps.c
> @@ -28,13 +28,12 @@
>  
>  /* Return an array of useful/necessary hardware capability names.  */
>  const struct r_strlenpair *
> -_dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
> -		      size_t *max_capstrlen)
> +_dl_important_hwcaps (size_t *sz, size_t *max_capstrlen)
>  {
>    uint64_t hwcap_mask = GET_HWCAP_MASK();
>    /* Determine how many important bits are set.  */
>    uint64_t masked = GLRO(dl_hwcap) & hwcap_mask;
> -  size_t cnt = platform != NULL;
> +  size_t cnt = GLRO (dl_platform) != NULL;
>    size_t n, m;
>    size_t total;
>    struct r_strlenpair *result;

Ok.

> @@ -60,10 +59,10 @@ _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz,
>  	masked ^= 1ULL << n;
>  	++m;
>        }
> -  if (platform != NULL)
> +  if (GLRO (dl_platform) != NULL)
>      {
> -      temp[m].str = platform;
> -      temp[m].len = platform_len;
> +      temp[m].str = GLRO (dl_platform);
> +      temp[m].len = GLRO (dl_platformlen);
>        ++m;
>      }
>  

Ok.

> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 0c8fa72c4d..f3201e7c14 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -697,8 +697,7 @@ _dl_init_paths (const char *llp, const char *source)
>  
>  #ifdef SHARED
>    /* Get the capabilities.  */
> -  capstr = _dl_important_hwcaps (GLRO(dl_platform), GLRO(dl_platformlen),
> -				 &ncapstr, &max_capstrlen);
> +  capstr = _dl_important_hwcaps (&ncapstr, &max_capstrlen);
>  #endif
>  
>    /* First set up the rest of the default search directory entries.  */

Ok.

> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index 510a2f6841..382eeb9be0 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -1071,12 +1071,12 @@ extern void _dl_show_auxv (void) attribute_hidden;
>     other.  */
>  extern char *_dl_next_ld_env_entry (char ***position) attribute_hidden;
>  
> -/* Return an array with the names of the important hardware capabilities.  */
> -extern const struct r_strlenpair *_dl_important_hwcaps (const char *platform,
> -							size_t paltform_len,
> -							size_t *sz,
> -							size_t *max_capstrlen)
> -     attribute_hidden;
> +/* Return an array with the names of the important hardware
> +   capabilities.  The length of the array is written to *SZ, and the
> +   maximum of all strings length is written to *MAX_CAPSTRLEN.  */
> +const struct r_strlenpair *_dl_important_hwcaps (size_t *sz,
> +						 size_t *max_capstrlen)
> +  attribute_hidden;
>  
>  /* Look up NAME in ld.so.cache and return the file name stored there,
>     or null if none is found.  Caller must free returned string.  */
> 

Ok.