[PATCH v5 2/4] Add system-wide tunables: cache ld.so.cache

[DJ Delorie]

Yury Khrustalev <yury.khrustalev@arm.com> writes:
> This is such an extensive change. Is the existing functionality covered
> with any tests?

It's the dynamic loader.  Every program in the testsuite is a test ;-)

>> +#include <startup.h>
>
> Nit: this include is not needed, it seems

Removed.

>> +#include <sys/types.h>
> Nit: this one as well seems to be unnecessary

Removed.

>> @@ -506,14 +597,7 @@ _dl_load_cache_lookup (const char *name)
>>  void
>>  _dl_unload_cache (void)
>>  {
>> -  if (cache != NULL && cache != (struct cache_file *) -1)
>> -    {
>> -      __munmap (cache, cachesize);
>> -      cache = NULL;
>> -    }
>> -#ifdef SHARED
>> -  /* This marks the glibc_hwcaps_priorities array as out-of-date.  */
>> -  glibc_hwcaps_priorities_length = 0;
>> -#endif
>> +  /* Functionality is no longer needed, but kept for internal ABI for
>> +     now.  */
>
> This seems really suspicious. This is used in 2 places and presumably
> some effect is expected from calling this function and now nothing will
> happen?

The whole point of caching the data is to, well, cache the data.  It's
not freed unless it's being replaced by data from a known-good cache
file.  This was requested by Carlos, so I'll defer to him to justify it.
IIRC at least part of the reason was to ensure that dlopen() had valid
cache data to work with, even if the file is corrupted while the program
was running.