Re: [PATCH dwarves 1/4] dutil: return ELF section name when looked up by index

[Stephen Brennan <stephen.s.brennan@oracle.com>]

Alan Maguire <alan.maguire@oracle.com> writes:

> On 12/09/2024 20:08, Stephen Brennan wrote:
>
> nit; needs a commit message, e.g. ELF section names will be used in
> DATASEC encoding; as well as getting the section, optionally retrieve
> the name.
>
>> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
>
> I think one small thing needs fixing below, but
>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
>> ---
>>  dutil.c | 9 ++++++++-
>>  dutil.h | 2 +-
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/dutil.c b/dutil.c
>> index 97c4474..4e83d59 100644
>> --- a/dutil.c
>> +++ b/dutil.c
>> @@ -207,13 +207,20 @@ Elf_Scn *elf_section_by_name(Elf *elf, GElf_Shdr *shp, const char *name, size_t
>>  	return sec;
>>  }
>>  
>> -Elf_Scn *elf_section_by_idx(Elf *elf, GElf_Shdr *shp, int idx)
>> +Elf_Scn *elf_section_by_idx(Elf *elf, GElf_Shdr *shp, int idx, const char **name_out)
>>  {
>>  	Elf_Scn *sec;
>> +	size_t str_idx;
>>  
>>  	sec = elf_getscn(elf, idx);
>>  	if (sec)
>>  		gelf_getshdr(sec, shp);
>> +
>> +	if (name_out) {
>
> nit; before we were directly returning sec, so if it was NULL that was
> for the caller to deal with; now though we're driving on assuming it was
> non-NULL here. So I'd suggest changing the above to be something like
>
> 	sec = elf_getscn(elf, idx);
> 	if (!sec)
> 		return NULL;
> 	if (!gelf_getshhdr(sec, shp))
> 		return NULL;
> 	if (name_out) {
>

D'oh! You're exactly right. Thanks!

>> +		if (elf_getshdrstrndx(elf, &str_idx))
>> +			return NULL;
>> +		*name_out = elf_strptr(elf, str_idx, shp->sh_name);
>> +	}
>>  	return sec;
>>  }
>>  
>> diff --git a/dutil.h b/dutil.h
>> index 335a17c..ff78aa6 100644
>> --- a/dutil.h
>> +++ b/dutil.h
>> @@ -328,7 +328,7 @@ void *zalloc(const size_t size);
>>  
>>  Elf_Scn *elf_section_by_name(Elf *elf, GElf_Shdr *shp, const char *name, size_t *index);
>>  
>> -Elf_Scn *elf_section_by_idx(Elf *elf, GElf_Shdr *shp, int idx);
>> +Elf_Scn *elf_section_by_idx(Elf *elf, GElf_Shdr *shp, int idx, const char **name_out);
>>  
>>  #ifndef SHT_GNU_ATTRIBUTES
>>  /* Just a way to check if we're using an old elfutils version */