Hi Martin,

On Mon, 2021-04-19 at 15:18 +0200, Martin Liška wrote:
> I made similar changes to binutils some time ago and I would like to
> come up with the same function for elfutils. Note that current
> construct
> is quite error prone, I found for instance these 2 bad usages:
> 
> diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
> index 88b5211d..b6de3510 100644
> --- a/libdwfl/relocate.c
> +++ b/libdwfl/relocate.c
> @@ -518,7 +518,7 @@ relocate_section (Dwfl_Module *mod, Elf
> *relocated, const GElf_Ehdr *ehdr,
>         Nothing to do here.  */
>      return DWFL_E_NOERROR;
>  
> -  if (strncmp (tname, ".zdebug", strlen ("zdebug")) == 0)
> +  if (strncmp (tname, ".zdebug", strlen (".zdebug")) == 0)
>      elf_compress_gnu (tscn, 0, 0);
>  
>    if ((tshdr->sh_flags & SHF_COMPRESSED) != 0)
> @@ -539,7 +539,7 @@ relocate_section (Dwfl_Module *mod, Elf
> *relocated, const GElf_Ehdr *ehdr,
>    if (sname == NULL)
>      return DWFL_E_LIBELF;
>  
> -  if (strncmp (sname, ".zdebug", strlen ("zdebug")) == 0)
> +  if (strncmp (sname, ".zdebug", strlen (".zdebug")) == 0)
>      elf_compress_gnu (scn, 0, 0);
>  
>    if ((shdr->sh_flags & SHF_COMPRESSED) != 0)

Urgh. Thanks for finding this!

> I'm not convinced about function declaration in system.h. Is it a
> proper location?

Yes, I think it is.

> And the function is not used in debuginfod/debuginfod-client.c and
> debuginfod/debuginfod.cxx.
> I would need another decl for these, am I right?

I think they both can simply include system.h (might want to double
check the -I search path in debuginfod/Makefile.am) because system.h
should be stand-alone, you don't need to link to anything else.

Maybe for debuginfod.cxx there is a better C++ way for strings. But if
it uses C strings, then it could also simply include system.h.

Thanks,

Mark

Reply via email to