Re: [PATCH 23/28] elf: Unify old and new format cache handling code in ld.so

[Florian Weimer <fweimer@redhat.com>]

* Adhemerval Zanella via Libc-alpha:

>> +/* Compute the address of the element INDEX of the array at LIBS.
>> +   Conceptually, this is &LIBS[INDEX], but use ENTRY_SIZE for the size
>> +   of *LIBS.  */
>> +static inline const struct file_entry *
>> +_dl_cache_file_entry (const struct file_entry *libs, size_t entry_size,
>> +		      size_t index)
>> +{
>> +  return (const void *) libs + index * entry_size;
>> +}
>> +
>
> Although pointer arithmetic with void *is valid for gcc, I think we should 
> use uintptr_t here.

Do you mean char *?  It will need another cast for the result.  I don't
think it clarifies matters.

>> +{
>> +  /* Used by the HWCAP check in the struct file_entry_new case.  */
>> +  uint64_t platform = _dl_string_platform (GLRO (dl_platform));
>> +  if (platform != (uint64_t) -1)
>> +    platform = 1ULL << platform;
>
> Maybe UINT64_C(1) << platform?

1ULL needs to be at least 64 bits, so this should be fine.

>> +
>> +		      best = string_table + lib->value;
>> +
>> +		      if (flags == GLRO (dl_correct_cache_id))
>> +			/* We've found an exact match for the shared
>> +			   object and no general `ELF' release.  Stop
>
> Should we replace the grave accent with apostrophe in this case?

I want to minimize the changes in this refactoring.

>> diff --git a/sysdeps/generic/dl-cache.h b/sysdeps/generic/dl-cache.h
>> index b154740da9..fec209509d 100644
>> --- a/sysdeps/generic/dl-cache.h
>> +++ b/sysdeps/generic/dl-cache.h
>> @@ -66,8 +66,8 @@
>>  */
>>  struct file_entry
>>  {
>> -  int flags;		/* This is 1 for an ELF library.  */
>> -  unsigned int key, value; /* String table indices.  */
>> +  int32_t flags;		/* This is 1 for an ELF library.  */
>> +  uint32_t key, value;		/* String table indices.  */
>>  };
>>  
>
> Ok, although not sure if our code guidelines allows multiple fields in the
> same line.

Likewise.

I'll assume your Reviewed-by: stands and push this separately.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill