Re: [PATCH] Move 4 libdwfl nested functions.

[Chih-Hung Hsieh <chh at google dot com>]

Mark, Roland,

How do you think about this patch?
Is this an acceptable way to convert nested functions?


On Fri, Sep 18, 2015 at 11:25 AM, Chih-Hung Hsieh <chh@google.com> wrote:

> Now they should compile with clang.
>
> Used local variables are passed to new file scope functions
> as constant parameters, or pointers, or embedded in a
> 'state' structure.
>
> One simple function "report" is changed to a macro.
> It triggers a gcc false positive -Werror=maybe-uninitialized,
> so the local variables are explicitly initialized.
>
> Signed-off-by: Chih-Hung Hsieh <chh@google.com>
> ---
>  libdwfl/ChangeLog              |  16 ++++
>  libdwfl/linux-kernel-modules.c | 120 +++++++++++++++------------
>  libdwfl/relocate.c             | 184
> ++++++++++++++++++++++-------------------
>  3 files changed, 182 insertions(+), 138 deletions(-)
>
> diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
> index 0ab386f..b9f42a1 100644
> --- a/libdwfl/ChangeLog
> +++ b/libdwfl/ChangeLog
> @@ -1,3 +1,19 @@
> +2015-09-18  Chih-Hung Hsieh  <chh@google.com>
> +
> +       * relocate.c (relocate_section): Move nested function "relocate"
> +       to file scope, pass all used local variables as constants.
> +       Move nested "check_badreltype" to file scope, pass addresses of
> +       updated used local variables.
> +       * linux-kernel-modules.c (intuit_kernel_bounds): Move nested
> function
> +       "read_address" to file scope, pass all updated local variables in
> +       a state structure.
> +       * linux-kernel-modules.c (dwfl_linux_kernel_find_elf): Move nested
> +       function "subst_name" to file scope, pass all used local variables
> +       as constants.
> +       * linux-kernel-modules.c (dwfl_linux_kernel_report_kernel): Replace
> +       simple nested function "report" with a macro. Work around gcc
> static
> +       analysis error -Werror=maybe-uninitialized.
> +
>  2015-09-09  Chih-Hung Hsieh  <chh@google.com>
>             Mark Wielaard  <mjw@redhat.com>
>
> diff --git a/libdwfl/linux-kernel-modules.c
> b/libdwfl/linux-kernel-modules.c
> index 236e2cd..dafe893 100644
> --- a/libdwfl/linux-kernel-modules.c
> +++ b/libdwfl/linux-kernel-modules.c
> @@ -440,46 +440,55 @@ dwfl_linux_kernel_report_offline (Dwfl *dwfl, const
> char *release,
>  INTDEF (dwfl_linux_kernel_report_offline)
>
>
> +/* State of read_address used by intuit_kernel_bounds. */
> +struct read_address_state {
> +  FILE *f;
> +  char *line;
> +  size_t linesz;
> +  size_t n;
> +  char *p;
> +  const char *type;
> +};
> +
> +static inline bool
> +read_address (struct read_address_state *state, Dwarf_Addr *addr)
> +{
> +  if ((state->n = getline (&state->line, &state->linesz, state->f)) < 1 ||
> +      state->line[state->n - 2] == ']')
> +    return false;
> +  *addr = strtoull (state->line, &state->p, 16);
> +  state->p += strspn (state->p, " \t");
> +  state->type = strsep (&state->p, " \t\n");
> +  if (state->type == NULL)
> +    return false;
> +  return state->p != NULL && state->p != state->line;
> +}
> +
> +
>  /* Grovel around to guess the bounds of the runtime kernel image.  */
>  static int
>  intuit_kernel_bounds (Dwarf_Addr *start, Dwarf_Addr *end, Dwarf_Addr
> *notes)
>  {
> -  FILE *f = fopen (KSYMSFILE, "r");
> -  if (f == NULL)
> +  struct read_address_state state = { NULL, NULL, 0, 0, NULL, NULL };
> +
> +  state.f = fopen (KSYMSFILE, "r");
> +  if (state.f == NULL)
>      return errno;
>
> -  (void) __fsetlocking (f, FSETLOCKING_BYCALLER);
> +  (void) __fsetlocking (state.f, FSETLOCKING_BYCALLER);
>
>    *notes = 0;
>
> -  char *line = NULL;
> -  size_t linesz = 0;
> -  size_t n;
> -  char *p = NULL;
> -  const char *type;
> -
> -  inline bool read_address (Dwarf_Addr *addr)
> -  {
> -    if ((n = getline (&line, &linesz, f)) < 1 || line[n - 2] == ']')
> -      return false;
> -    *addr = strtoull (line, &p, 16);
> -    p += strspn (p, " \t");
> -    type = strsep (&p, " \t\n");
> -    if (type == NULL)
> -      return false;
> -    return p != NULL && p != line;
> -  }
> -
>    int result;
>    do
> -    result = read_address (start) ? 0 : -1;
> -  while (result == 0 && strchr ("TtRr", *type) == NULL);
> +    result = read_address (&state, start) ? 0 : -1;
> +  while (result == 0 && strchr ("TtRr", *state.type) == NULL);
>
>    if (result == 0)
>      {
>        *end = *start;
> -      while (read_address (end))
> -       if (*notes == 0 && !strcmp (p, "__start_notes\n"))
> +      while (read_address (&state, end))
> +       if (*notes == 0 && !strcmp (state.p, "__start_notes\n"))
>           *notes = *end;
>
>        Dwarf_Addr round_kernel = sysconf (_SC_PAGE_SIZE);
> @@ -489,12 +498,12 @@ intuit_kernel_bounds (Dwarf_Addr *start, Dwarf_Addr
> *end, Dwarf_Addr *notes)
>        if (*start >= *end || *end - *start < round_kernel)
>         result = -1;
>      }
> -  free (line);
> +  free (state.line);
>
>    if (result == -1)
> -    result = ferror_unlocked (f) ? errno : ENOEXEC;
> +    result = ferror_unlocked (state.f) ? errno : ENOEXEC;
>
> -  fclose (f);
> +  fclose (state.f);
>
>    return result;
>  }
> @@ -619,12 +628,11 @@ check_module_notes (Dwfl_Module *mod)
>  int
>  dwfl_linux_kernel_report_kernel (Dwfl *dwfl)
>  {
> -  Dwarf_Addr start;
> -  Dwarf_Addr end;
> -  inline Dwfl_Module *report (void)
> -    {
> -      return INTUSE(dwfl_report_module) (dwfl, KERNEL_MODNAME, start,
> end);
> -    }
> +  Dwarf_Addr start = 0;
> +  Dwarf_Addr end = 0;
> +
> +  #define report() \
> +    (INTUSE(dwfl_report_module) (dwfl, KERNEL_MODNAME, start, end))
>
>    /* This is a bit of a kludge.  If we already reported the kernel,
>       don't bother figuring it out again--it never changes.  */
> @@ -657,6 +665,29 @@ dwfl_linux_kernel_report_kernel (Dwfl *dwfl)
>  INTDEF (dwfl_linux_kernel_report_kernel)
>
>
> +static inline bool
> +subst_name (char from, char to,
> +            const char * const module_name,
> +            char * const alternate_name,
> +            const size_t namelen)
> +{
> +  const char *n = memchr (module_name, from, namelen);
> +  if (n == NULL)
> +    return false;
> +  char *a = mempcpy (alternate_name, module_name, n - module_name);
> +  *a++ = to;
> +  ++n;
> +  const char *p;
> +  while ((p = memchr (n, from, namelen - (n - module_name))) != NULL)
> +    {
> +      a = mempcpy (a, n, p - n);
> +      *a++ = to;
> +      n = p + 1;
> +    }
> +  memcpy (a, n, namelen - (n - module_name) + 1);
> +  return true;
> +}
> +
>  /* Dwfl_Callbacks.find_elf for the running Linux kernel and its modules.
> */
>
>  int
> @@ -713,25 +744,8 @@ dwfl_linux_kernel_find_elf (Dwfl_Module *mod,
>        free (modulesdir[0]);
>        return ENOMEM;
>      }
> -  inline bool subst_name (char from, char to)
> -    {
> -      const char *n = memchr (module_name, from, namelen);
> -      if (n == NULL)
> -       return false;
> -      char *a = mempcpy (alternate_name, module_name, n - module_name);
> -      *a++ = to;
> -      ++n;
> -      const char *p;
> -      while ((p = memchr (n, from, namelen - (n - module_name))) != NULL)
> -       {
> -         a = mempcpy (a, n, p - n);
> -         *a++ = to;
> -         n = p + 1;
> -       }
> -      memcpy (a, n, namelen - (n - module_name) + 1);
> -      return true;
> -    }
> -  if (!subst_name ('-', '_') && !subst_name ('_', '-'))
> +  if (!subst_name ('-', '_', module_name, alternate_name, namelen) &&
> +      !subst_name ('_', '-', module_name, alternate_name, namelen))
>      alternate_name[0] = '\0';
>
>    FTSENT *f;
> diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
> index e102e1e..2dc6737 100644
> --- a/libdwfl/relocate.c
> +++ b/libdwfl/relocate.c
> @@ -277,77 +277,18 @@ resolve_symbol (Dwfl_Module *referer, struct
> reloc_symtab_cache *symtab,
>    return DWFL_E_RELUNDEF;
>  }
>
> +/* Apply one relocation.  Returns true for any invalid data.  */
>  static Dwfl_Error
> -relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
> -                 size_t shstrndx, struct reloc_symtab_cache *reloc_symtab,
> -                 Elf_Scn *scn, GElf_Shdr *shdr,
> -                 Elf_Scn *tscn, bool debugscn, bool partial)
> +relocate (Dwfl_Module * const mod,
> +          Elf * const relocated,
> +          struct reloc_symtab_cache * const reloc_symtab,
> +          Elf_Data * const tdata,
> +          const GElf_Ehdr * const ehdr,
> +          GElf_Addr offset,
> +          const GElf_Sxword *addend,
> +          int rtype,
> +          int symndx)
>  {
> -  /* First, fetch the name of the section these relocations apply to.  */
> -  GElf_Shdr tshdr_mem;
> -  GElf_Shdr *tshdr = gelf_getshdr (tscn, &tshdr_mem);
> -  const char *tname = elf_strptr (relocated, shstrndx, tshdr->sh_name);
> -  if (tname == NULL)
> -    return DWFL_E_LIBELF;
> -
> -  if (unlikely (tshdr->sh_type == SHT_NOBITS) || unlikely (tshdr->sh_size
> == 0))
> -    /* No contents to relocate.  */
> -    return DWFL_E_NOERROR;
> -
> -  if (debugscn && ! ebl_debugscn_p (mod->ebl, tname))
> -    /* This relocation section is not for a debugging section.
> -       Nothing to do here.  */
> -    return DWFL_E_NOERROR;
> -
> -  /* Fetch the section data that needs the relocations applied.  */
> -  Elf_Data *tdata = elf_rawdata (tscn, NULL);
> -  if (tdata == NULL)
> -    return DWFL_E_LIBELF;
> -
> -  /* If either the section that needs the relocation applied, or the
> -     section that the relocations come from overlap one of the ehdrs,
> -     shdrs or phdrs data then we refuse to do the relocations.  It
> -     isn't illegal for ELF section data to overlap the header data,
> -     but updating the (relocation) data might corrupt the in-memory
> -     libelf headers causing strange corruptions or errors.  */
> -  size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT);
> -  if (unlikely (shdr->sh_offset < ehsize
> -               || tshdr->sh_offset < ehsize))
> -    return DWFL_E_BADELF;
> -
> -  GElf_Off shdrs_start = ehdr->e_shoff;
> -  size_t shnums;
> -  if (elf_getshdrnum (relocated, &shnums) < 0)
> -    return DWFL_E_LIBELF;
> -  /* Overflows will have been checked by elf_getshdrnum/get|rawdata.  */
> -  size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT);
> -  GElf_Off shdrs_end = shdrs_start + shnums * shentsize;
> -  if (unlikely ((shdrs_start < shdr->sh_offset + shdr->sh_size
> -                && shdr->sh_offset < shdrs_end)
> -               || (shdrs_start < tshdr->sh_offset + tshdr->sh_size
> -                   && tshdr->sh_offset < shdrs_end)))
> -    return DWFL_E_BADELF;
> -
> -  GElf_Off phdrs_start = ehdr->e_phoff;
> -  size_t phnums;
> -  if (elf_getphdrnum (relocated, &phnums) < 0)
> -    return DWFL_E_LIBELF;
> -  if (phdrs_start != 0 && phnums != 0)
> -    {
> -      /* Overflows will have been checked by elf_getphdrnum/get|rawdata.
> */
> -      size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1,
> EV_CURRENT);
> -      GElf_Off phdrs_end = phdrs_start + phnums * phentsize;
> -      if (unlikely ((phdrs_start < shdr->sh_offset + shdr->sh_size
> -                    && shdr->sh_offset < phdrs_end)
> -                   || (phdrs_start < tshdr->sh_offset + tshdr->sh_size
> -                       && tshdr->sh_offset < phdrs_end)))
> -       return DWFL_E_BADELF;
> -    }
> -
> -  /* Apply one relocation.  Returns true for any invalid data.  */
> -  Dwfl_Error relocate (GElf_Addr offset, const GElf_Sxword *addend,
> -                      int rtype, int symndx)
> -  {
>      /* First see if this is a reloc we can handle.
>         If we are skipping it, don't bother resolving the symbol.  */
>
> @@ -482,7 +423,89 @@ relocate_section (Dwfl_Module *mod, Elf *relocated,
> const GElf_Ehdr *ehdr,
>
>      /* We have applied this relocation!  */
>      return DWFL_E_NOERROR;
> -  }
> +}
> +
> +static inline void
> +check_badreltype (bool *first_badreltype,
> +                  Dwfl_Module *mod,
> +                  Dwfl_Error *result)
> +{
> +  if (*first_badreltype)
> +    {
> +       *first_badreltype = false;
> +       if (ebl_get_elfmachine (mod->ebl) == EM_NONE)
> +          /* This might be because ebl_openbackend failed to find
> +             any libebl_CPU.so library.  Diagnose that clearly.  */
> +          *result = DWFL_E_UNKNOWN_MACHINE;
> +     }
> +}
> +
> +static Dwfl_Error
> +relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
> +                 size_t shstrndx, struct reloc_symtab_cache *reloc_symtab,
> +                 Elf_Scn *scn, GElf_Shdr *shdr,
> +                 Elf_Scn *tscn, bool debugscn, bool partial)
> +{
> +  /* First, fetch the name of the section these relocations apply to.  */
> +  GElf_Shdr tshdr_mem;
> +  GElf_Shdr *tshdr = gelf_getshdr (tscn, &tshdr_mem);
> +  const char *tname = elf_strptr (relocated, shstrndx, tshdr->sh_name);
> +  if (tname == NULL)
> +    return DWFL_E_LIBELF;
> +
> +  if (unlikely (tshdr->sh_type == SHT_NOBITS) || unlikely (tshdr->sh_size
> == 0))
> +    /* No contents to relocate.  */
> +    return DWFL_E_NOERROR;
> +
> +  if (debugscn && ! ebl_debugscn_p (mod->ebl, tname))
> +    /* This relocation section is not for a debugging section.
> +       Nothing to do here.  */
> +    return DWFL_E_NOERROR;
> +
> +  /* Fetch the section data that needs the relocations applied.  */
> +  Elf_Data *tdata = elf_rawdata (tscn, NULL);
> +  if (tdata == NULL)
> +    return DWFL_E_LIBELF;
> +
> +  /* If either the section that needs the relocation applied, or the
> +     section that the relocations come from overlap one of the ehdrs,
> +     shdrs or phdrs data then we refuse to do the relocations.  It
> +     isn't illegal for ELF section data to overlap the header data,
> +     but updating the (relocation) data might corrupt the in-memory
> +     libelf headers causing strange corruptions or errors.  */
> +  size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT);
> +  if (unlikely (shdr->sh_offset < ehsize
> +               || tshdr->sh_offset < ehsize))
> +    return DWFL_E_BADELF;
> +
> +  GElf_Off shdrs_start = ehdr->e_shoff;
> +  size_t shnums;
> +  if (elf_getshdrnum (relocated, &shnums) < 0)
> +    return DWFL_E_LIBELF;
> +  /* Overflows will have been checked by elf_getshdrnum/get|rawdata.  */
> +  size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT);
> +  GElf_Off shdrs_end = shdrs_start + shnums * shentsize;
> +  if (unlikely ((shdrs_start < shdr->sh_offset + shdr->sh_size
> +                && shdr->sh_offset < shdrs_end)
> +               || (shdrs_start < tshdr->sh_offset + tshdr->sh_size
> +                   && tshdr->sh_offset < shdrs_end)))
> +    return DWFL_E_BADELF;
> +
> +  GElf_Off phdrs_start = ehdr->e_phoff;
> +  size_t phnums;
> +  if (elf_getphdrnum (relocated, &phnums) < 0)
> +    return DWFL_E_LIBELF;
> +  if (phdrs_start != 0 && phnums != 0)
> +    {
> +      /* Overflows will have been checked by elf_getphdrnum/get|rawdata.
> */
> +      size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1,
> EV_CURRENT);
> +      GElf_Off phdrs_end = phdrs_start + phnums * phentsize;
> +      if (unlikely ((phdrs_start < shdr->sh_offset + shdr->sh_size
> +                    && shdr->sh_offset < phdrs_end)
> +                   || (phdrs_start < tshdr->sh_offset + tshdr->sh_size
> +                       && tshdr->sh_offset < phdrs_end)))
> +       return DWFL_E_BADELF;
> +    }
>
>    /* Fetch the relocation section and apply each reloc in it.  */
>    Elf_Data *reldata = elf_getdata (scn, NULL);
> @@ -491,17 +514,6 @@ relocate_section (Dwfl_Module *mod, Elf *relocated,
> const GElf_Ehdr *ehdr,
>
>    Dwfl_Error result = DWFL_E_NOERROR;
>    bool first_badreltype = true;
> -  inline void check_badreltype (void)
> -  {
> -    if (first_badreltype)
> -      {
> -       first_badreltype = false;
> -       if (ebl_get_elfmachine (mod->ebl) == EM_NONE)
> -         /* This might be because ebl_openbackend failed to find
> -            any libebl_CPU.so library.  Diagnose that clearly.  */
> -         result = DWFL_E_UNKNOWN_MACHINE;
> -      }
> -  }
>
>    size_t sh_entsize
>      = gelf_fsize (relocated, shdr->sh_type == SHT_REL ? ELF_T_REL :
> ELF_T_RELA,
> @@ -514,10 +526,11 @@ relocate_section (Dwfl_Module *mod, Elf *relocated,
> const GElf_Ehdr *ehdr,
>         GElf_Rel rel_mem, *r = gelf_getrel (reldata, relidx, &rel_mem);
>         if (r == NULL)
>           return DWFL_E_LIBELF;
> -       result = relocate (r->r_offset, NULL,
> +       result = relocate (mod, relocated, reloc_symtab, tdata, ehdr,
> +                          r->r_offset, NULL,
>                            GELF_R_TYPE (r->r_info),
>                            GELF_R_SYM (r->r_info));
> -       check_badreltype ();
> +       check_badreltype (&first_badreltype, mod, &result);
>         if (partial)
>           switch (result)
>             {
> @@ -543,10 +556,11 @@ relocate_section (Dwfl_Module *mod, Elf *relocated,
> const GElf_Ehdr *ehdr,
>                                                &rela_mem);
>         if (r == NULL)
>           return DWFL_E_LIBELF;
> -       result = relocate (r->r_offset, &r->r_addend,
> +       result = relocate (mod, relocated, reloc_symtab, tdata, ehdr,
> +                          r->r_offset, &r->r_addend,
>                            GELF_R_TYPE (r->r_info),
>                            GELF_R_SYM (r->r_info));
> -       check_badreltype ();
> +       check_badreltype (&first_badreltype, mod, &result);
>         if (partial)
>           switch (result)
>             {
> --
> 2.6.0.rc0.131.gf624c3d
>
>

Mark, Roland,

How do you think about this patch?

Is this an acceptable way to convert nested functions?

 

On Fri, Sep 18, 2015 at 11:25 AM, Chih-Hung Hsieh <chh@google.com> wrote:

Now they should compile with clang.

Used local variables are passed to new file scope functions
as constant parameters, or pointers, or embedded in a
'state' structure.

One simple function "report" is changed to a macro.
It triggers a gcc false positive -Werror=maybe-uninitialized,
so the local variables are explicitly initialized.

Signed-off-by: Chih-Hung Hsieh <chh@google.com>
---
 libdwfl/ChangeLog              |  16 ++++
 libdwfl/linux-kernel-modules.c | 120 +++++++++++++++------------
 libdwfl/relocate.c             | 184 ++++++++++++++++++++++-------------------
 3 files changed, 182 insertions(+), 138 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 0ab386f..b9f42a1 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,19 @@
+2015-09-18  Chih-Hung Hsieh  <chh@google.com>
+
+       * relocate.c (relocate_section): Move nested function "relocate"
+       to file scope, pass all used local variables as constants.
+       Move nested "check_badreltype" to file scope, pass addresses of
+       updated used local variables.
+       * linux-kernel-modules.c (intuit_kernel_bounds): Move nested function
+       "read_address" to file scope, pass all updated local variables in
+       a state structure.
+       * linux-kernel-modules.c (dwfl_linux_kernel_find_elf): Move nested
+       function "subst_name" to file scope, pass all used local variables
+       as constants.
+       * linux-kernel-modules.c (dwfl_linux_kernel_report_kernel): Replace
+       simple nested function "report" with a macro. Work around gcc static
+       analysis error -Werror=maybe-uninitialized.
+
 2015-09-09  Chih-Hung Hsieh  <chh@google.com>
            Mark Wielaard  <mjw@redhat.com>

diff --git a/libdwfl/linux-kernel-modules.c b/libdwfl/linux-kernel-modules.c
index 236e2cd..dafe893 100644
--- a/libdwfl/linux-kernel-modules.c
+++ b/libdwfl/linux-kernel-modules.c
@@ -440,46 +440,55 @@ dwfl_linux_kernel_report_offline (Dwfl *dwfl, const char *release,
 INTDEF (dwfl_linux_kernel_report_offline)


+/* State of read_address used by intuit_kernel_bounds. */
+struct read_address_state {
+  FILE *f;
+  char *line;
+  size_t linesz;
+  size_t n;
+  char *p;
+  const char *type;
+};
+
+static inline bool
+read_address (struct read_address_state *state, Dwarf_Addr *addr)
+{
+  if ((state->n = getline (&state->line, &state->linesz, state->f)) < 1 ||
+      state->line[state->n - 2] == ']')
+    return false;
+  *addr = strtoull (state->line, &state->p, 16);
+  state->p += strspn (state->p, " \t");
+  state->type = strsep (&state->p, " \t\n");
+  if (state->type == NULL)
+    return false;
+  return state->p != NULL && state->p != state->line;
+}
+
+
 /* Grovel around to guess the bounds of the runtime kernel image.  */
 static int
 intuit_kernel_bounds (Dwarf_Addr *start, Dwarf_Addr *end, Dwarf_Addr *notes)
 {
-  FILE *f = fopen (KSYMSFILE, "r");
-  if (f == NULL)
+  struct read_address_state state = { NULL, NULL, 0, 0, NULL, NULL };
+
+  state.f = fopen (KSYMSFILE, "r");
+  if (state.f == NULL)
     return errno;

-  (void) __fsetlocking (f, FSETLOCKING_BYCALLER);
+  (void) __fsetlocking (state.f, FSETLOCKING_BYCALLER);

   *notes = 0;

-  char *line = NULL;
-  size_t linesz = 0;
-  size_t n;
-  char *p = NULL;
-  const char *type;
-
-  inline bool read_address (Dwarf_Addr *addr)
-  {
-    if ((n = getline (&line, &linesz, f)) < 1 || line[n - 2] == ']')
-      return false;
-    *addr = strtoull (line, &p, 16);
-    p += strspn (p, " \t");
-    type = strsep (&p, " \t\n");
-    if (type == NULL)
-      return false;
-    return p != NULL && p != line;
-  }
-
   int result;
   do
-    result = read_address (start) ? 0 : -1;
-  while (result == 0 && strchr ("TtRr", *type) == NULL);
+    result = read_address (&state, start) ? 0 : -1;
+  while (result == 0 && strchr ("TtRr", *state.type) == NULL);

   if (result == 0)
     {
       *end = *start;
-      while (read_address (end))
-       if (*notes == 0 && !strcmp (p, "__start_notes\n"))
+      while (read_address (&state, end))
+       if (*notes == 0 && !strcmp (state.p, "__start_notes\n"))
          *notes = *end;

       Dwarf_Addr round_kernel = sysconf (_SC_PAGE_SIZE);
@@ -489,12 +498,12 @@ intuit_kernel_bounds (Dwarf_Addr *start, Dwarf_Addr *end, Dwarf_Addr *notes)
       if (*start >= *end || *end - *start < round_kernel)
        result = -1;
     }
-  free (line);
+  free (state.line);

   if (result == -1)
-    result = ferror_unlocked (f) ? errno : ENOEXEC;
+    result = ferror_unlocked (state.f) ? errno : ENOEXEC;

-  fclose (f);
+  fclose (state.f);

   return result;
 }
@@ -619,12 +628,11 @@ check_module_notes (Dwfl_Module *mod)
 int
 dwfl_linux_kernel_report_kernel (Dwfl *dwfl)
 {
-  Dwarf_Addr start;
-  Dwarf_Addr end;
-  inline Dwfl_Module *report (void)
-    {
-      return INTUSE(dwfl_report_module) (dwfl, KERNEL_MODNAME, start, end);
-    }
+  Dwarf_Addr start = 0;
+  Dwarf_Addr end = 0;
+
+  #define report() \
+    (INTUSE(dwfl_report_module) (dwfl, KERNEL_MODNAME, start, end))

   /* This is a bit of a kludge.  If we already reported the kernel,
      don't bother figuring it out again--it never changes.  */
@@ -657,6 +665,29 @@ dwfl_linux_kernel_report_kernel (Dwfl *dwfl)
 INTDEF (dwfl_linux_kernel_report_kernel)


+static inline bool
+subst_name (char from, char to,
+            const char * const module_name,
+            char * const alternate_name,
+            const size_t namelen)
+{
+  const char *n = memchr (module_name, from, namelen);
+  if (n == NULL)
+    return false;
+  char *a = mempcpy (alternate_name, module_name, n - module_name);
+  *a++ = to;
+  ++n;
+  const char *p;
+  while ((p = memchr (n, from, namelen - (n - module_name))) != NULL)
+    {
+      a = mempcpy (a, n, p - n);
+      *a++ = to;
+      n = p + 1;
+    }
+  memcpy (a, n, namelen - (n - module_name) + 1);
+  return true;
+}
+
 /* Dwfl_Callbacks.find_elf for the running Linux kernel and its modules.  */

 int
@@ -713,25 +744,8 @@ dwfl_linux_kernel_find_elf (Dwfl_Module *mod,
       free (modulesdir[0]);
       return ENOMEM;
     }
-  inline bool subst_name (char from, char to)
-    {
-      const char *n = memchr (module_name, from, namelen);
-      if (n == NULL)
-       return false;
-      char *a = mempcpy (alternate_name, module_name, n - module_name);
-      *a++ = to;
-      ++n;
-      const char *p;
-      while ((p = memchr (n, from, namelen - (n - module_name))) != NULL)
-       {
-         a = mempcpy (a, n, p - n);
-         *a++ = to;
-         n = p + 1;
-       }
-      memcpy (a, n, namelen - (n - module_name) + 1);
-      return true;
-    }
-  if (!subst_name ('-', '_') && !subst_name ('_', '-'))
+  if (!subst_name ('-', '_', module_name, alternate_name, namelen) &&
+      !subst_name ('_', '-', module_name, alternate_name, namelen))
     alternate_name[0] = '\0';

   FTSENT *f;
diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
index e102e1e..2dc6737 100644
--- a/libdwfl/relocate.c
+++ b/libdwfl/relocate.c
@@ -277,77 +277,18 @@ resolve_symbol (Dwfl_Module *referer, struct reloc_symtab_cache *symtab,
   return DWFL_E_RELUNDEF;
 }

+/* Apply one relocation.  Returns true for any invalid data.  */
 static Dwfl_Error
-relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
-                 size_t shstrndx, struct reloc_symtab_cache *reloc_symtab,
-                 Elf_Scn *scn, GElf_Shdr *shdr,
-                 Elf_Scn *tscn, bool debugscn, bool partial)
+relocate (Dwfl_Module * const mod,
+          Elf * const relocated,
+          struct reloc_symtab_cache * const reloc_symtab,
+          Elf_Data * const tdata,
+          const GElf_Ehdr * const ehdr,
+          GElf_Addr offset,
+          const GElf_Sxword *addend,
+          int rtype,
+          int symndx)
 {
-  /* First, fetch the name of the section these relocations apply to.  */
-  GElf_Shdr tshdr_mem;
-  GElf_Shdr *tshdr = gelf_getshdr (tscn, &tshdr_mem);
-  const char *tname = elf_strptr (relocated, shstrndx, tshdr->sh_name);
-  if (tname == NULL)
-    return DWFL_E_LIBELF;
-
-  if (unlikely (tshdr->sh_type == SHT_NOBITS) || unlikely (tshdr->sh_size == 0))
-    /* No contents to relocate.  */
-    return DWFL_E_NOERROR;
-
-  if (debugscn && ! ebl_debugscn_p (mod->ebl, tname))
-    /* This relocation section is not for a debugging section.
-       Nothing to do here.  */
-    return DWFL_E_NOERROR;
-
-  /* Fetch the section data that needs the relocations applied.  */
-  Elf_Data *tdata = elf_rawdata (tscn, NULL);
-  if (tdata == NULL)
-    return DWFL_E_LIBELF;
-
-  /* If either the section that needs the relocation applied, or the
-     section that the relocations come from overlap one of the ehdrs,
-     shdrs or phdrs data then we refuse to do the relocations.  It
-     isn't illegal for ELF section data to overlap the header data,
-     but updating the (relocation) data might corrupt the in-memory
-     libelf headers causing strange corruptions or errors.  */
-  size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT);
-  if (unlikely (shdr->sh_offset < ehsize
-               || tshdr->sh_offset < ehsize))
-    return DWFL_E_BADELF;
-
-  GElf_Off shdrs_start = ehdr->e_shoff;
-  size_t shnums;
-  if (elf_getshdrnum (relocated, &shnums) < 0)
-    return DWFL_E_LIBELF;
-  /* Overflows will have been checked by elf_getshdrnum/get|rawdata.  */
-  size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT);
-  GElf_Off shdrs_end = shdrs_start + shnums * shentsize;
-  if (unlikely ((shdrs_start < shdr->sh_offset + shdr->sh_size
-                && shdr->sh_offset < shdrs_end)
-               || (shdrs_start < tshdr->sh_offset + tshdr->sh_size
-                   && tshdr->sh_offset < shdrs_end)))
-    return DWFL_E_BADELF;
-
-  GElf_Off phdrs_start = ehdr->e_phoff;
-  size_t phnums;
-  if (elf_getphdrnum (relocated, &phnums) < 0)
-    return DWFL_E_LIBELF;
-  if (phdrs_start != 0 && phnums != 0)
-    {
-      /* Overflows will have been checked by elf_getphdrnum/get|rawdata.  */
-      size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1, EV_CURRENT);
-      GElf_Off phdrs_end = phdrs_start + phnums * phentsize;
-      if (unlikely ((phdrs_start < shdr->sh_offset + shdr->sh_size
-                    && shdr->sh_offset < phdrs_end)
-                   || (phdrs_start < tshdr->sh_offset + tshdr->sh_size
-                       && tshdr->sh_offset < phdrs_end)))
-       return DWFL_E_BADELF;
-    }
-
-  /* Apply one relocation.  Returns true for any invalid data.  */
-  Dwfl_Error relocate (GElf_Addr offset, const GElf_Sxword *addend,
-                      int rtype, int symndx)
-  {
     /* First see if this is a reloc we can handle.
        If we are skipping it, don't bother resolving the symbol.  */

@@ -482,7 +423,89 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,

     /* We have applied this relocation!  */
     return DWFL_E_NOERROR;
-  }
+}
+
+static inline void
+check_badreltype (bool *first_badreltype,
+                  Dwfl_Module *mod,
+                  Dwfl_Error *result)
+{
+  if (*first_badreltype)
+    {
+       *first_badreltype = false;
+       if (ebl_get_elfmachine (mod->ebl) == EM_NONE)
+          /* This might be because ebl_openbackend failed to find
+             any libebl_CPU.so library.  Diagnose that clearly.  */
+          *result = DWFL_E_UNKNOWN_MACHINE;
+     }
+}
+
+static Dwfl_Error
+relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
+                 size_t shstrndx, struct reloc_symtab_cache *reloc_symtab,
+                 Elf_Scn *scn, GElf_Shdr *shdr,
+                 Elf_Scn *tscn, bool debugscn, bool partial)
+{
+  /* First, fetch the name of the section these relocations apply to.  */
+  GElf_Shdr tshdr_mem;
+  GElf_Shdr *tshdr = gelf_getshdr (tscn, &tshdr_mem);
+  const char *tname = elf_strptr (relocated, shstrndx, tshdr->sh_name);
+  if (tname == NULL)
+    return DWFL_E_LIBELF;
+
+  if (unlikely (tshdr->sh_type == SHT_NOBITS) || unlikely (tshdr->sh_size == 0))
+    /* No contents to relocate.  */
+    return DWFL_E_NOERROR;
+
+  if (debugscn && ! ebl_debugscn_p (mod->ebl, tname))
+    /* This relocation section is not for a debugging section.
+       Nothing to do here.  */
+    return DWFL_E_NOERROR;
+
+  /* Fetch the section data that needs the relocations applied.  */
+  Elf_Data *tdata = elf_rawdata (tscn, NULL);
+  if (tdata == NULL)
+    return DWFL_E_LIBELF;
+
+  /* If either the section that needs the relocation applied, or the
+     section that the relocations come from overlap one of the ehdrs,
+     shdrs or phdrs data then we refuse to do the relocations.  It
+     isn't illegal for ELF section data to overlap the header data,
+     but updating the (relocation) data might corrupt the in-memory
+     libelf headers causing strange corruptions or errors.  */
+  size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT);
+  if (unlikely (shdr->sh_offset < ehsize
+               || tshdr->sh_offset < ehsize))
+    return DWFL_E_BADELF;
+
+  GElf_Off shdrs_start = ehdr->e_shoff;
+  size_t shnums;
+  if (elf_getshdrnum (relocated, &shnums) < 0)
+    return DWFL_E_LIBELF;
+  /* Overflows will have been checked by elf_getshdrnum/get|rawdata.  */
+  size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT);
+  GElf_Off shdrs_end = shdrs_start + shnums * shentsize;
+  if (unlikely ((shdrs_start < shdr->sh_offset + shdr->sh_size
+                && shdr->sh_offset < shdrs_end)
+               || (shdrs_start < tshdr->sh_offset + tshdr->sh_size
+                   && tshdr->sh_offset < shdrs_end)))
+    return DWFL_E_BADELF;
+
+  GElf_Off phdrs_start = ehdr->e_phoff;
+  size_t phnums;
+  if (elf_getphdrnum (relocated, &phnums) < 0)
+    return DWFL_E_LIBELF;
+  if (phdrs_start != 0 && phnums != 0)
+    {
+      /* Overflows will have been checked by elf_getphdrnum/get|rawdata.  */
+      size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1, EV_CURRENT);
+      GElf_Off phdrs_end = phdrs_start + phnums * phentsize;
+      if (unlikely ((phdrs_start < shdr->sh_offset + shdr->sh_size
+                    && shdr->sh_offset < phdrs_end)
+                   || (phdrs_start < tshdr->sh_offset + tshdr->sh_size
+                       && tshdr->sh_offset < phdrs_end)))
+       return DWFL_E_BADELF;
+    }

   /* Fetch the relocation section and apply each reloc in it.  */
   Elf_Data *reldata = elf_getdata (scn, NULL);
@@ -491,17 +514,6 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,

   Dwfl_Error result = DWFL_E_NOERROR;
   bool first_badreltype = true;
-  inline void check_badreltype (void)
-  {
-    if (first_badreltype)
-      {
-       first_badreltype = false;
-       if (ebl_get_elfmachine (mod->ebl) == EM_NONE)
-         /* This might be because ebl_openbackend failed to find
-            any libebl_CPU.so library.  Diagnose that clearly.  */
-         result = DWFL_E_UNKNOWN_MACHINE;
-      }
-  }

   size_t sh_entsize
     = gelf_fsize (relocated, shdr->sh_type == SHT_REL ? ELF_T_REL : ELF_T_RELA,
@@ -514,10 +526,11 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
        GElf_Rel rel_mem, *r = gelf_getrel (reldata, relidx, &rel_mem);
        if (r == NULL)
          return DWFL_E_LIBELF;
-       result = relocate (r->r_offset, NULL,
+       result = relocate (mod, relocated, reloc_symtab, tdata, ehdr,
+                          r->r_offset, NULL,
                           GELF_R_TYPE (r->r_info),
                           GELF_R_SYM (r->r_info));
-       check_badreltype ();
+       check_badreltype (&first_badreltype, mod, &result);
        if (partial)
          switch (result)
            {
@@ -543,10 +556,11 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr,
                                               &rela_mem);
        if (r == NULL)
          return DWFL_E_LIBELF;
-       result = relocate (r->r_offset, &r->r_addend,
+       result = relocate (mod, relocated, reloc_symtab, tdata, ehdr,
+                          r->r_offset, &r->r_addend,
                           GELF_R_TYPE (r->r_info),
                           GELF_R_SYM (r->r_info));
-       check_badreltype ();
+       check_badreltype (&first_badreltype, mod, &result);
        if (partial)
          switch (result)
            {
--
2.6.0.rc0.131.gf624c3d