[PATCH] Remove nested functions from ELF iterators

[Colin Watson]

Part five.  In this case, the hooks already had void * arguments for
user data, so it was just a matter of moving functions up to the top
level, although I did take the opportunity to add typedefs for the hook
types.  No verbose ChangeLog this time since there are no interface
changes.  Size changes:

  DOWN: obj/amd64-efi/grub-core/elf.mod (6888 > 6880) - change: -8
  DOWN: obj/i386-coreboot/grub-core/elf.mod (4424 > 4408) - change: -16
  DOWN: obj/i386-efi/grub-core/elf.mod (4536 > 4528) - change: -8
  DOWN: obj/i386-ieee1275/grub-core/elf.mod (4424 > 4408) - change: -16
  DOWN: obj/i386-pc/grub-core/elf.mod (4424 > 4408) - change: -16

=== modified file 'ChangeLog'
--- ChangeLog   2013-01-16 12:41:16 +0000
+++ ChangeLog   2013-01-16 14:26:36 +0000
@@ -1,3 +1,7 @@
+2013-01-16  Colin Watson  <address@hidden>
+
+       Remove nested functions from ELF iterators.
+
 2013-01-16  Vladimir Serbinenko  <address@hidden>
 
        Improve bidi handling in entry editor.

=== modified file 'grub-core/kern/elf.c'
--- grub-core/kern/elf.c        2012-02-26 16:28:05 +0000
+++ grub-core/kern/elf.c        2013-01-16 14:22:44 +0000
@@ -149,8 +149,7 @@ grub_elf32_load_phdrs (grub_elf_t elf, c
 grub_err_t
 grub_elf32_phdr_iterate (grub_elf_t elf,
                         const char *filename,
-                        int NESTED_FUNC_ATTR (*hook) (grub_elf_t, Elf32_Phdr 
*, void *),
-                        void *hook_arg)
+                        grub_elf32_phdr_iterate_hook_t hook, void *hook_arg)
 {
   Elf32_Phdr *phdrs;
   unsigned int i;
@@ -177,48 +176,58 @@ grub_elf32_phdr_iterate (grub_elf_t elf,
   return grub_errno;
 }
 
+struct grub_elf32_size_ctx
+{
+  Elf32_Addr segments_start, segments_end;
+  int nr_phdrs;
+  grub_uint32_t curr_align;
+};
+
+/* Run through the program headers to calculate the total memory size we
+ * should claim.  */
+static int
+grub_elf32_calcsize (grub_elf_t _elf  __attribute__ ((unused)),
+                    Elf32_Phdr *phdr, void *data)
+{
+  struct grub_elf32_size_ctx *ctx = data;
+
+  /* Only consider loadable segments.  */
+  if (phdr->p_type != PT_LOAD)
+    return 0;
+  ctx->nr_phdrs++;
+  if (phdr->p_paddr < ctx->segments_start)
+    ctx->segments_start = phdr->p_paddr;
+  if (phdr->p_paddr + phdr->p_memsz > ctx->segments_end)
+    ctx->segments_end = phdr->p_paddr + phdr->p_memsz;
+  if (ctx->curr_align < phdr->p_align)
+    ctx->curr_align = phdr->p_align;
+  return 0;
+}
+
 /* Calculate the amount of memory spanned by the segments.  */
 grub_size_t
 grub_elf32_size (grub_elf_t elf, const char *filename,
                 Elf32_Addr *base, grub_uint32_t *max_align)
 {
-  Elf32_Addr segments_start = (Elf32_Addr) -1;
-  Elf32_Addr segments_end = 0;
-  int nr_phdrs = 0;
-  grub_uint32_t curr_align = 1;
-
-  /* Run through the program headers to calculate the total memory size we
-   * should claim.  */
-  auto int NESTED_FUNC_ATTR calcsize (grub_elf_t _elf, Elf32_Phdr *phdr, void 
*_arg);
-  int NESTED_FUNC_ATTR calcsize (grub_elf_t _elf  __attribute__ ((unused)),
-                                Elf32_Phdr *phdr,
-                                void *_arg __attribute__ ((unused)))
-    {
-      /* Only consider loadable segments.  */
-      if (phdr->p_type != PT_LOAD)
-       return 0;
-      nr_phdrs++;
-      if (phdr->p_paddr < segments_start)
-       segments_start = phdr->p_paddr;
-      if (phdr->p_paddr + phdr->p_memsz > segments_end)
-       segments_end = phdr->p_paddr + phdr->p_memsz;
-      if (curr_align < phdr->p_align)
-       curr_align = phdr->p_align;
-      return 0;
-    }
+  struct grub_elf32_size_ctx ctx = {
+    .segments_start = (Elf32_Addr) -1,
+    .segments_end = 0,
+    .nr_phdrs = 0,
+    .curr_align = 1
+  };
 
-  grub_elf32_phdr_iterate (elf, filename, calcsize, 0);
+  grub_elf32_phdr_iterate (elf, filename, grub_elf32_calcsize, &ctx);
 
   if (base)
     *base = 0;
 
-  if (nr_phdrs == 0)
+  if (ctx.nr_phdrs == 0)
     {
       grub_error (GRUB_ERR_BAD_OS, "no program headers present");
       return 0;
     }
 
-  if (segments_end < segments_start)
+  if (ctx.segments_end < ctx.segments_start)
     {
       /* Very bad addresses.  */
       grub_error (GRUB_ERR_BAD_OS, "bad program header load addresses");
@@ -226,76 +235,87 @@ grub_elf32_size (grub_elf_t elf, const c
     }
 
   if (base)
-    *base = segments_start;
+    *base = ctx.segments_start;
   if (max_align)
-    *max_align = curr_align;
-  return segments_end - segments_start;
+    *max_align = ctx.curr_align;
+  return ctx.segments_end - ctx.segments_start;
 }
 
-/* Load every loadable segment into memory specified by `_load_hook'.  */
-grub_err_t
-grub_elf32_load (grub_elf_t _elf, const char *filename,
-                grub_elf32_load_hook_t _load_hook,
-                grub_addr_t *base, grub_size_t *size)
+struct grub_elf32_load_ctx
 {
-  grub_addr_t load_base = (grub_addr_t) -1ULL;
-  grub_size_t load_size = 0;
-  grub_err_t err;
+  const char *filename;
+  grub_elf32_load_hook_t load_hook;
+  grub_addr_t load_base;
+  grub_size_t load_size;
+};
 
-  auto int NESTED_FUNC_ATTR grub_elf32_load_segment (grub_elf_t elf, 
Elf32_Phdr *phdr, void *hook);
-  int NESTED_FUNC_ATTR grub_elf32_load_segment (grub_elf_t elf, Elf32_Phdr 
*phdr, void *hook)
-  {
-    grub_elf32_load_hook_t load_hook = (grub_elf32_load_hook_t) hook;
-    grub_addr_t load_addr;
-    int do_load = 1;
-
-    load_addr = phdr->p_paddr;
-    if (load_hook && load_hook (phdr, &load_addr, &do_load))
-      return 1;
+static int
+grub_elf32_load_segment (grub_elf_t elf, Elf32_Phdr *phdr, void *data)
+{
+  struct grub_elf32_load_ctx *ctx = data;
+  grub_addr_t load_addr;
+  int do_load = 1;
+
+  load_addr = phdr->p_paddr;
+  if (ctx->load_hook && ctx->load_hook (phdr, &load_addr, &do_load))
+    return 1;
 
-    if (! do_load)
-      return 0;
+  if (! do_load)
+    return 0;
 
-    if (load_addr < load_base)
-      load_base = load_addr;
+  if (load_addr < ctx->load_base)
+    ctx->load_base = load_addr;
 
-    grub_dprintf ("elf", "Loading segment at 0x%llx, size 0x%llx\n",
-                 (unsigned long long) load_addr,
-                 (unsigned long long) phdr->p_memsz);
+  grub_dprintf ("elf", "Loading segment at 0x%llx, size 0x%llx\n",
+               (unsigned long long) load_addr,
+               (unsigned long long) phdr->p_memsz);
 
-    if (grub_file_seek (elf->file, phdr->p_offset) == (grub_off_t) -1)
-      return grub_errno;
+  if (grub_file_seek (elf->file, phdr->p_offset) == (grub_off_t) -1)
+    return grub_errno;
 
-    if (phdr->p_filesz)
-      {
-       grub_ssize_t read;
-       read = grub_file_read (elf->file, (void *) load_addr, phdr->p_filesz);
-       if (read != (grub_ssize_t) phdr->p_filesz)
-         {
-           /* XXX How can we free memory from `load_hook'? */
-           if (!grub_errno)
-             grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file 
%s"),
-                         filename);
-           return grub_errno;
-         }
-      }
-
-    if (phdr->p_filesz < phdr->p_memsz)
-      grub_memset ((void *) (long) (load_addr + phdr->p_filesz),
-                  0, phdr->p_memsz - phdr->p_filesz);
+  if (phdr->p_filesz)
+    {
+      grub_ssize_t read;
+      read = grub_file_read (elf->file, (void *) load_addr, phdr->p_filesz);
+      if (read != (grub_ssize_t) phdr->p_filesz)
+       {
+         /* XXX How can we free memory from `ctx->load_hook'? */
+         if (!grub_errno)
+           grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file 
%s"),
+                       ctx->filename);
+         return grub_errno;
+       }
+    }
 
-    load_size += phdr->p_memsz;
+  if (phdr->p_filesz < phdr->p_memsz)
+    grub_memset ((void *) (long) (load_addr + phdr->p_filesz),
+                0, phdr->p_memsz - phdr->p_filesz);
 
-    return 0;
-  }
+  ctx->load_size += phdr->p_memsz;
+
+  return 0;
+}
+
+/* Load every loadable segment into memory specified by `_load_hook'.  */
+grub_err_t
+grub_elf32_load (grub_elf_t elf, const char *filename,
+                grub_elf32_load_hook_t load_hook,
+                grub_addr_t *base, grub_size_t *size)
+{
+  struct grub_elf32_load_ctx ctx = {
+    .filename = filename,
+    .load_hook = load_hook,
+    .load_base = (grub_addr_t) -1ULL,
+    .load_size = 0
+  };
+  grub_err_t err;
 
-  err = grub_elf32_phdr_iterate (_elf, filename,
-                                grub_elf32_load_segment, _load_hook);
+  err = grub_elf32_phdr_iterate (elf, filename, grub_elf32_load_segment, &ctx);
 
   if (base)
-    *base = load_base;
+    *base = ctx.load_base;
   if (size)
-    *size = load_size;
+    *size = ctx.load_size;
 
   return err;
 }
@@ -339,8 +359,7 @@ grub_elf64_load_phdrs (grub_elf_t elf, c
 grub_err_t
 grub_elf64_phdr_iterate (grub_elf_t elf,
                         const char *filename,
-                        int NESTED_FUNC_ATTR (*hook) (grub_elf_t, Elf64_Phdr 
*, void *),
-                        void *hook_arg)
+                        grub_elf64_phdr_iterate_hook_t hook, void *hook_arg)
 {
   Elf64_Phdr *phdrs;
   unsigned int i;
@@ -367,48 +386,58 @@ grub_elf64_phdr_iterate (grub_elf_t elf,
   return grub_errno;
 }
 
+struct grub_elf64_size_ctx
+{
+  Elf64_Addr segments_start, segments_end;
+  int nr_phdrs;
+  grub_uint64_t curr_align;
+};
+
+/* Run through the program headers to calculate the total memory size we
+ * should claim.  */
+static int
+grub_elf64_calcsize (grub_elf_t _elf  __attribute__ ((unused)),
+                    Elf64_Phdr *phdr, void *data)
+{
+  struct grub_elf64_size_ctx *ctx = data;
+
+  /* Only consider loadable segments.  */
+  if (phdr->p_type != PT_LOAD)
+    return 0;
+  ctx->nr_phdrs++;
+  if (phdr->p_paddr < ctx->segments_start)
+    ctx->segments_start = phdr->p_paddr;
+  if (phdr->p_paddr + phdr->p_memsz > ctx->segments_end)
+    ctx->segments_end = phdr->p_paddr + phdr->p_memsz;
+  if (ctx->curr_align < phdr->p_align)
+    ctx->curr_align = phdr->p_align;
+  return 0;
+}
+
 /* Calculate the amount of memory spanned by the segments.  */
 grub_size_t
 grub_elf64_size (grub_elf_t elf, const char *filename,
                 Elf64_Addr *base, grub_uint64_t *max_align)
 {
-  Elf64_Addr segments_start = (Elf64_Addr) -1;
-  Elf64_Addr segments_end = 0;
-  int nr_phdrs = 0;
-  grub_uint64_t curr_align = 1;
-
-  /* Run through the program headers to calculate the total memory size we
-   * should claim.  */
-  auto int NESTED_FUNC_ATTR calcsize (grub_elf_t _elf, Elf64_Phdr *phdr, void 
*_arg);
-  int NESTED_FUNC_ATTR calcsize (grub_elf_t _elf  __attribute__ ((unused)),
-                                Elf64_Phdr *phdr,
-                                void *_arg __attribute__ ((unused)))
-    {
-      /* Only consider loadable segments.  */
-      if (phdr->p_type != PT_LOAD)
-       return 0;
-      nr_phdrs++;
-      if (phdr->p_paddr < segments_start)
-       segments_start = phdr->p_paddr;
-      if (phdr->p_paddr + phdr->p_memsz > segments_end)
-       segments_end = phdr->p_paddr + phdr->p_memsz;
-      if (curr_align < phdr->p_align)
-       curr_align = phdr->p_align;
-      return 0;
-    }
+  struct grub_elf64_size_ctx ctx = {
+    .segments_start = (Elf64_Addr) -1,
+    .segments_end = 0,
+    .nr_phdrs = 0,
+    .curr_align = 1
+  };
 
-  grub_elf64_phdr_iterate (elf, filename, calcsize, 0);
+  grub_elf64_phdr_iterate (elf, filename, grub_elf64_calcsize, &ctx);
 
   if (base)
     *base = 0;
 
-  if (nr_phdrs == 0)
+  if (ctx.nr_phdrs == 0)
     {
       grub_error (GRUB_ERR_BAD_OS, "no program headers present");
       return 0;
     }
 
-  if (segments_end < segments_start)
+  if (ctx.segments_end < ctx.segments_start)
     {
       /* Very bad addresses.  */
       grub_error (GRUB_ERR_BAD_OS, "bad program header load addresses");
@@ -416,77 +445,87 @@ grub_elf64_size (grub_elf_t elf, const c
     }
 
   if (base)
-    *base = segments_start;
+    *base = ctx.segments_start;
   if (max_align)
-    *max_align = curr_align;
-  return segments_end - segments_start;
+    *max_align = ctx.curr_align;
+  return ctx.segments_end - ctx.segments_start;
 }
 
-/* Load every loadable segment into memory specified by `_load_hook'.  */
-grub_err_t
-grub_elf64_load (grub_elf_t _elf, const char *filename,
-                grub_elf64_load_hook_t _load_hook,
-                grub_addr_t *base, grub_size_t *size)
+struct grub_elf64_load_ctx
 {
-  grub_addr_t load_base = (grub_addr_t) -1ULL;
-  grub_size_t load_size = 0;
-  grub_err_t err;
+  const char *filename;
+  grub_elf64_load_hook_t load_hook;
+  grub_addr_t load_base;
+  grub_size_t load_size;
+};
 
-  auto int NESTED_FUNC_ATTR grub_elf64_load_segment (grub_elf_t elf, 
Elf64_Phdr *phdr,
-                                                    void *hook);
-  int NESTED_FUNC_ATTR grub_elf64_load_segment (grub_elf_t elf, Elf64_Phdr 
*phdr, void *hook)
-  {
-    grub_elf64_load_hook_t load_hook = (grub_elf64_load_hook_t) hook;
-    grub_addr_t load_addr;
-    int do_load = 1;
-
-    load_addr = phdr->p_paddr;
-    if (load_hook && load_hook (phdr, &load_addr, &do_load))
-      return 1;
+static int
+grub_elf64_load_segment (grub_elf_t elf, Elf64_Phdr *phdr, void *data)
+{
+  struct grub_elf64_load_ctx *ctx = data;
+  grub_addr_t load_addr;
+  int do_load = 1;
+
+  load_addr = phdr->p_paddr;
+  if (ctx->load_hook && ctx->load_hook (phdr, &load_addr, &do_load))
+    return 1;
 
-    if (! do_load)
-      return 0;
+  if (! do_load)
+    return 0;
 
-    if (load_addr < load_base)
-      load_base = load_addr;
+  if (load_addr < ctx->load_base)
+    ctx->load_base = load_addr;
 
-    grub_dprintf ("elf", "Loading segment at 0x%llx, size 0x%llx\n",
-                 (unsigned long long) load_addr,
-                 (unsigned long long) phdr->p_memsz);
+  grub_dprintf ("elf", "Loading segment at 0x%llx, size 0x%llx\n",
+               (unsigned long long) load_addr,
+               (unsigned long long) phdr->p_memsz);
 
-    if (grub_file_seek (elf->file, phdr->p_offset) == (grub_off_t) -1)
-      return grub_errno;
+  if (grub_file_seek (elf->file, phdr->p_offset) == (grub_off_t) -1)
+    return grub_errno;
 
-    if (phdr->p_filesz)
-      {
-       grub_ssize_t read;
-       read = grub_file_read (elf->file, (void *) load_addr, phdr->p_filesz);
-       if (read != (grub_ssize_t) phdr->p_filesz)
-          {
-           /* XXX How can we free memory from `load_hook'?  */
-           if (!grub_errno)
-             grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file 
%s"),
-                         filename);
-           return grub_errno;
-          }
-      }
-
-    if (phdr->p_filesz < phdr->p_memsz)
-      grub_memset ((void *) (long) (load_addr + phdr->p_filesz),
-                  0, phdr->p_memsz - phdr->p_filesz);
+  if (phdr->p_filesz)
+    {
+      grub_ssize_t read;
+      read = grub_file_read (elf->file, (void *) load_addr, phdr->p_filesz);
+      if (read != (grub_ssize_t) phdr->p_filesz)
+       {
+         /* XXX How can we free memory from `ctx->load_hook'?  */
+         if (!grub_errno)
+           grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file 
%s"),
+                       ctx->filename);
+         return grub_errno;
+       }
+    }
 
-    load_size += phdr->p_memsz;
+  if (phdr->p_filesz < phdr->p_memsz)
+    grub_memset ((void *) (long) (load_addr + phdr->p_filesz),
+                0, phdr->p_memsz - phdr->p_filesz);
 
-    return 0;
-  }
+  ctx->load_size += phdr->p_memsz;
+
+  return 0;
+}
+
+/* Load every loadable segment into memory specified by `_load_hook'.  */
+grub_err_t
+grub_elf64_load (grub_elf_t elf, const char *filename,
+                grub_elf64_load_hook_t load_hook,
+                grub_addr_t *base, grub_size_t *size)
+{
+  struct grub_elf64_load_ctx ctx = {
+    .filename = filename,
+    .load_hook = load_hook,
+    .load_base = (grub_addr_t) -1ULL,
+    .load_size = 0
+  };
+  grub_err_t err;
 
-  err = grub_elf64_phdr_iterate (_elf, filename,
-                                grub_elf64_load_segment, _load_hook);
+  err = grub_elf64_phdr_iterate (elf, filename, grub_elf64_load_segment, &ctx);
 
   if (base)
-    *base = load_base;
+    *base = ctx.load_base;
   if (size)
-    *size = load_size;
+    *size = ctx.load_size;
 
   return err;
 }

=== modified file 'grub-core/loader/i386/bsd.c'
--- grub-core/loader/i386/bsd.c 2013-01-15 12:02:35 +0000
+++ grub-core/loader/i386/bsd.c 2013-01-16 14:11:56 +0000
@@ -1311,7 +1311,7 @@ grub_bsd_load_aout (grub_file_t file, co
                         bss_size);
 }
 
-static int NESTED_FUNC_ATTR
+static int
 grub_bsd_elf32_size_hook (grub_elf_t elf __attribute__ ((unused)),
                          Elf32_Phdr *phdr, void *arg __attribute__ ((unused)))
 {
@@ -1353,7 +1353,7 @@ grub_bsd_elf32_hook (Elf32_Phdr * phdr,
   return GRUB_ERR_NONE;
 }
 
-static int NESTED_FUNC_ATTR
+static int
 grub_bsd_elf64_size_hook (grub_elf_t elf __attribute__ ((unused)),
                          Elf64_Phdr *phdr, void *arg __attribute__ ((unused)))
 {

=== modified file 'include/grub/elfload.h'
--- include/grub/elfload.h      2012-02-08 18:26:01 +0000
+++ include/grub/elfload.h      2013-01-16 14:14:20 +0000
@@ -41,6 +41,11 @@ typedef grub_err_t (*grub_elf32_load_hoo
 typedef grub_err_t (*grub_elf64_load_hook_t)
   (Elf64_Phdr *phdr, grub_addr_t *addr, int *load);
 
+typedef int (*grub_elf32_phdr_iterate_hook_t)
+  (grub_elf_t elf, Elf32_Phdr *phdr, void *arg);
+typedef int (*grub_elf64_phdr_iterate_hook_t)
+  (grub_elf_t elf, Elf64_Phdr *phdr, void *arg);
+
 grub_elf_t grub_elf_open (const char *);
 grub_elf_t grub_elf_file (grub_file_t file, const char *filename);
 grub_err_t grub_elf_close (grub_elf_t);
@@ -63,12 +68,10 @@ grub_err_t grub_elf64_load (grub_elf_t,
 grub_err_t
 grub_elf32_phdr_iterate (grub_elf_t elf,
                         const char *filename,
-                        int NESTED_FUNC_ATTR (*hook) (grub_elf_t, Elf32_Phdr 
*, void *),
-                        void *hook_arg);
+                        grub_elf32_phdr_iterate_hook_t hook, void *hook_arg);
 grub_err_t
 grub_elf64_phdr_iterate (grub_elf_t elf,
                         const char *filename,
-                        int NESTED_FUNC_ATTR (*hook) (grub_elf_t, Elf64_Phdr 
*, void *),
-                        void *hook_arg);
+                        grub_elf64_phdr_iterate_hook_t hook, void *hook_arg);
 
 #endif /* ! GRUB_ELFLOAD_HEADER */


Thanks,

-- 
Colin Watson                                       address@hidden