[SLOF] [PATCH slof 03/13] elf: Compile with -Wextra

[Alexey Kardashevskiy]

On 28/01/2021 02:18, Thomas Huth wrote:
> On 27/01/2021 09.57, Alexey Kardashevskiy wrote:
>> -Wextra enables a bunch of rather useful checks which this fixes.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
>> ---
> [...]
>> diff --git a/lib/libelf/elf32.c b/lib/libelf/elf32.c
>> index 64ea386a96da..b213a6f9ca17 100644
>> --- a/lib/libelf/elf32.c
>> +++ b/lib/libelf/elf32.c
>> @@ -212,13 +212,13 @@ elf_byteswap_header32(void *file_addr)
>>    * file.
>>    * @return Return -1 on error, size of file otherwise.
>>    */
>> -long elf_get_file_size32(const void *buffer, const long buffer_size)
>> +long elf_get_file_size32(const void *buffer, const unsigned long 
>> buffer_size)
>>   {
>>       const struct ehdr32 *ehdr = (const struct ehdr32 *) buffer;
>>       const uint8_t *buffer_end = buffer + buffer_size;
>>       const struct phdr32 *phdr;
>>       const struct shdr32 *shdr;
>> -    long elf_size = -1;
>> +    unsigned long elf_size = -1;
> 
> That looks kind of ugly, since the return type of the function is signed 
> again later...

True.

> Maybe it's better to cast in the MAX() function instead?

gcc happily casts the result of "+" to a signed type.


It is easier to do this before returning so I'll post it soon:


if (elf_size > 0 && (unsigned long) elf_size > buffer_size)
         return -1;


> 
>> diff --git a/lib/libelf/elf64.c b/lib/libelf/elf64.c
>> index 0f302679f784..eb4cc3a7eab3 100644
>> --- a/lib/libelf/elf64.c
>> +++ b/lib/libelf/elf64.c
>> @@ -389,7 +389,7 @@ elf_apply_all_rela64(void *file_addr, signed long 
>> offset, struct shdr64 *shdrs,
>>       struct rela *relaentry;
>>       struct sym64 *symtabentry;
>>       uint32_t symbolidx;
>> -    int i;
>> +    unsigned i;
>>       /* If the referenced section has not been allocated, then it has
>>        * not been loaded and thus does not need to be relocated. */
>> @@ -481,13 +481,13 @@ uint32_t elf_get_eflags_64(void *file_addr)
>>    * file.
>>    * @return Return -1 on error, size of file otherwise.
>>    */
>> -long elf_get_file_size64(const void *buffer, const long buffer_size)
>> +long elf_get_file_size64(const void *buffer, const unsigned long 
>> buffer_size)
>>   {
>>       const struct ehdr64 *ehdr = (const struct ehdr64 *) buffer;
>>       const uint8_t *buffer_end = buffer + buffer_size;
>>       const struct phdr64 *phdr;
>>       const struct shdr64 *shdr;
>> -    long elf_size = -1;
>> +    unsigned elf_size = (unsigned)-1;
> 
> Should that be unsigned *long* instead ?

Ouch, missed this one, thanks for spotting it.




> 
>   Thomas
> 

-- 
Alexey