grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/3] mkimage: refactor a bunch of section data into a stru


From: Daniel Kiper
Subject: Re: [PATCH v2 1/3] mkimage: refactor a bunch of section data into a struct.
Date: Wed, 21 Feb 2018 12:07:42 +0100
User-agent: Mutt/1.3.28i

On Tue, Feb 20, 2018 at 06:25:31PM -0500, Peter Jones wrote:
> This basically moves a bunch of the section information we pass around a
> lot into a struct, and passes a pointer to a single one of those
> instead.
>
> Because it's more convenient, it also puts the section_vaddresses
> calculations in locate_section(), which no longer returns the allocation
> for section_addresses directly either.

Could not you make this change in separate patch?

> This shouldn't change the binary file output or the "grub-mkimage -v"
> output in any way.
>
> Signed-off-by: Peter Jones <address@hidden>
> ---
>  util/grub-mkimagexx.c | 282 
> ++++++++++++++++++++++----------------------------
>  1 file changed, 123 insertions(+), 159 deletions(-)
>
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index a2bb05439f0..5c787ec56bf 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -84,6 +84,17 @@ struct fixup_block_list
>
>  #define ALIGN_ADDR(x) (ALIGN_UP((x), image_target->voidp_sizeof))
>
> +struct section_metadata
> +{
> +  Elf_Half num_sections;
> +  Elf_Shdr *sections;
> +  Elf_Addr *addresses;

s/addresses/addr/

> +  Elf_Addr *vaddresses;

s/vaddresses/vaddr/

> +  Elf_Half section_entsize;
> +  Elf_Shdr *symtab;
> +  const char *strtab;
> +};
> +
>  static int
>  is_relocatable (const struct grub_install_image_target_desc *image_target)
>  {
> @@ -499,9 +510,7 @@ SUFFIX (grub_mkimage_generate_elf) (const struct 
> grub_install_image_target_desc
>  /* Relocate symbols; note that this function overwrites the symbol table.
>     Return the address of a start symbol.  */
>  static Elf_Addr
> -SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr *sections,
> -                        Elf_Shdr *symtab_section, Elf_Addr 
> *section_addresses,
> -                        Elf_Half section_entsize, Elf_Half num_sections,
> +SUFFIX (relocate_symbols) (Elf_Ehdr *e, struct section_metadata *sdata,

I would do s/sdata/smdata/ or even s/sdata/smd/...

>                          void *jumpers, Elf_Addr jumpers_addr,
>                          Elf_Addr bss_start, Elf_Addr end,
>                          const struct grub_install_image_target_desc 
> *image_target)
> @@ -511,19 +520,18 @@ SUFFIX (relocate_symbols) (Elf_Ehdr *e, Elf_Shdr 
> *sections,
>    Elf_Addr start_address = (Elf_Addr) -1;
>    Elf_Sym *sym;
>    Elf_Word i;
> -  Elf_Shdr *strtab_section;
> -  const char *strtab;
> +  Elf_Shdr *symtab_section;
> +  const char *symtab;

This change does not seem to be logical part of this patch.

Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]