Re: [PATCH v3 07/32] elf: Disambiguate some failures in _dl_load_cache_lookup

[Joseph Myers <josmyers@redhat.com>]

On Thu, 7 Dec 2023, Florian Weimer wrote:

> diff --git a/elf/dl-cache.c b/elf/dl-cache.c
> index 804bf23222..a3eb960dac 100644
> --- a/elf/dl-cache.c
> +++ b/elf/dl-cache.c
> @@ -398,8 +398,8 @@ _dl_cache_libcmp (const char *p1, const char *p2)
>     may be unmapped at any time by a completing recursive dlopen and
>     this function must take care that it does not return references to
>     any data in the mapping.  */
> -char *
> -_dl_load_cache_lookup (const char *name)
> +bool
> +_dl_load_cache_lookup (const char *name, char **realname)

The comment above this function definition needs updating to describe the 
new semantics for return value and result stored in *REALNAME.

> -/* 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.  */
> -extern char *_dl_load_cache_lookup (const char *name) attribute_hidden;
> +/* Look up NAME in ld.so.cache.  Return false on memory allocation
> +   failure and do not change *REALNAME.  If lookup fails, return true
> +   and write a null pointer to *REALNAME.  If lookup suceeds, write a

"succeeds"

> +   copy of the full name to *REALNAME (which has to be freed by the
> +   caller).  */
> +bool _dl_load_cache_lookup (const char *name, char **realname)
> +  attribute_hidden __nonnull ((1, 2)) __attribute__ ((warn_unused_result));

Something like the comment here - though the two comments give different 
information, so don't lose the extra information on the definition unless 
you're confident it's no longer relevant.  However, the comment here 
doesn't actually say it returns true when lookup and allocation both 
succeed.

-- 
Joseph S. Myers
josmyers@redhat.com