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

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 <[email protected]>

I think one small thing needs fixing below, but

Reviewed-by: Alan Maguire <[email protected]>
> ---
>  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) {

...
> +		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 */