Re: [PATCH] Clean up struct grub_module_header

[Robert Millan]

It seems this has been leftover, probably because my response looked like
I objected to it (which I don't, it's perfectly reasonable cleanup).

I'm committing it.

On Sat, Aug 22, 2009 at 02:10:12AM -0400, Pavel Roskin wrote:
> `grub_module_header_types' should not be a field at all.  Move enum
> outside struct.
> 
> Keep using an 8-bit integer for `type', but make it unsigned.  Remove all
> byteswapping for `type', as it only has one byte.
> 
> Make `size' 32-bit, as grub-mkimage already assumes.  4 gigabytes
> should be enough for a module.  Use grub_host_to_target32() uniformly
> for the `size' field in all utilities.
> 
> ChangeLog:
> 
>       * include/grub/kernel.h (struct grub_module_header): Remove
>       `grub_module_header_types'.  Make `type' unsigned.  Make `size'
>       32-bit on all platforms.
>       * util/elf/grub-mkimage.c (load_modules): Treat `type' as an
>       8-bit field.  Use grub_host_to_target32() for `size'.
>       * util/i386/efi/grub-mkimage.c (make_mods_section): Likewise.
>       * util/i386/pc/grub-mkimage.c (generate_image): Likewise.
>       * util/sparc64/ieee1275/grub-mkimage.c (generate_image): Likewise.
> ---
>  include/grub/kernel.h                |   18 +++++++++---------
>  util/elf/grub-mkimage.c              |    6 +++---
>  util/i386/efi/grub-mkimage.c         |    4 ++--
>  util/i386/pc/grub-mkimage.c          |   12 ++++++------
>  util/sparc64/ieee1275/grub-mkimage.c |    8 ++++----
>  5 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/include/grub/kernel.h b/include/grub/kernel.h
> index 02bc276..75ec77c 100644
> --- a/include/grub/kernel.h
> +++ b/include/grub/kernel.h
> @@ -22,20 +22,20 @@
>  #include <grub/types.h>
>  #include <grub/symbol.h>
>  
> +enum
> +{
> +  OBJ_TYPE_ELF,
> +  OBJ_TYPE_MEMDISK,
> +  OBJ_TYPE_CONFIG
> +};
> +
>  /* The module header.  */
>  struct grub_module_header
>  {
>    /* The type of object.  */
> -  grub_int8_t type;
> -  enum
> -  {
> -    OBJ_TYPE_ELF,
> -    OBJ_TYPE_MEMDISK,
> -    OBJ_TYPE_CONFIG
> -  }  grub_module_header_types;
> -
> +  grub_uint8_t type;
>    /* The size of object (including this header).  */
> -  grub_target_size_t size;
> +  grub_uint32_t size;
>  };
>  
>  /* "gmim" (GRUB Module Info Magic).  */
> diff --git a/util/elf/grub-mkimage.c b/util/elf/grub-mkimage.c
> index 52bb639..5354272 100644
> --- a/util/elf/grub-mkimage.c
> +++ b/util/elf/grub-mkimage.c
> @@ -144,7 +144,7 @@ load_modules (grub_addr_t modbase, Elf32_Phdr *phdr, 
> const char *dir,
>        mod_size = grub_util_get_image_size (p->name);
>  
>        header = (struct grub_module_header *) (module_img + offset);
> -      header->type = grub_host_to_target32 (OBJ_TYPE_ELF);
> +      header->type = OBJ_TYPE_ELF;
>        header->size = grub_host_to_target32 (mod_size + sizeof (*header));
>  
>        grub_util_load_image (p->name, module_img + offset + sizeof (*header));
> @@ -157,8 +157,8 @@ load_modules (grub_addr_t modbase, Elf32_Phdr *phdr, 
> const char *dir,
>        struct grub_module_header *header;
>  
>        header = (struct grub_module_header *) (module_img + offset);
> -      header->type = grub_cpu_to_le32 (OBJ_TYPE_MEMDISK);
> -      header->size = grub_cpu_to_le32 (memdisk_size + sizeof (*header));
> +      header->type = OBJ_TYPE_MEMDISK;
> +      header->size = grub_host_to_target32 (memdisk_size + sizeof (*header));
>        offset += sizeof (*header);
>  
>        grub_util_load_image (memdisk_path, module_img + offset);
> diff --git a/util/i386/efi/grub-mkimage.c b/util/i386/efi/grub-mkimage.c
> index 466aa9f..29a823e 100644
> --- a/util/i386/efi/grub-mkimage.c
> +++ b/util/i386/efi/grub-mkimage.c
> @@ -683,8 +683,8 @@ make_mods_section (FILE *out, Elf_Addr current_address,
>        grub_util_info ("adding module %s", p->name);
>  
>        mod_size = grub_util_get_image_size (p->name);
> -      header.type = grub_cpu_to_le32 (OBJ_TYPE_ELF);
> -      header.size = grub_cpu_to_le32 (mod_size + sizeof (header));
> +      header.type = OBJ_TYPE_ELF;
> +      header.size = grub_host_to_target32 (mod_size + sizeof (header));
>  
>        mod_image = grub_util_read_image (p->name);
>  
> diff --git a/util/i386/pc/grub-mkimage.c b/util/i386/pc/grub-mkimage.c
> index b81392c..cf8f8f5 100644
> --- a/util/i386/pc/grub-mkimage.c
> +++ b/util/i386/pc/grub-mkimage.c
> @@ -153,8 +153,8 @@ generate_image (const char *dir, char *prefix, FILE *out, 
> char *mods[],
>  
>        header = (struct grub_module_header *) (kernel_img + offset);
>        memset (header, 0, sizeof (struct grub_module_header));
> -      header->type = grub_cpu_to_le32 (OBJ_TYPE_ELF);
> -      header->size = grub_cpu_to_le32 (mod_size + sizeof (*header));
> +      header->type = OBJ_TYPE_ELF;
> +      header->size = grub_host_to_target32 (mod_size + sizeof (*header));
>        offset += sizeof (*header);
>  
>        grub_util_load_image (p->name, kernel_img + offset);
> @@ -167,8 +167,8 @@ generate_image (const char *dir, char *prefix, FILE *out, 
> char *mods[],
>  
>        header = (struct grub_module_header *) (kernel_img + offset);
>        memset (header, 0, sizeof (struct grub_module_header));
> -      header->type = grub_cpu_to_le32 (OBJ_TYPE_MEMDISK);
> -      header->size = grub_cpu_to_le32 (memdisk_size + sizeof (*header));
> +      header->type = OBJ_TYPE_MEMDISK;
> +      header->size = grub_host_to_target32 (memdisk_size + sizeof (*header));
>        offset += sizeof (*header);
>  
>        grub_util_load_image (memdisk_path, kernel_img + offset);
> @@ -181,8 +181,8 @@ generate_image (const char *dir, char *prefix, FILE *out, 
> char *mods[],
>  
>        header = (struct grub_module_header *) (kernel_img + offset);
>        memset (header, 0, sizeof (struct grub_module_header));
> -      header->type = grub_cpu_to_le32 (OBJ_TYPE_CONFIG);
> -      header->size = grub_cpu_to_le32 (config_size + sizeof (*header));
> +      header->type = OBJ_TYPE_CONFIG;
> +      header->size = grub_host_to_target32 (config_size + sizeof (*header));
>        offset += sizeof (*header);
>  
>        grub_util_load_image (config_path, kernel_img + offset);
> diff --git a/util/sparc64/ieee1275/grub-mkimage.c 
> b/util/sparc64/ieee1275/grub-mkimage.c
> index 247f138..0a611da 100644
> --- a/util/sparc64/ieee1275/grub-mkimage.c
> +++ b/util/sparc64/ieee1275/grub-mkimage.c
> @@ -98,8 +98,8 @@ generate_image (const char *dir, const char *prefix, FILE 
> *out, char *mods[], ch
>        mod_size = grub_util_get_image_size (p->name);
>  
>        header = (struct grub_module_header *) (kernel_img + offset);
> -      header->type = grub_cpu_to_be32 (OBJ_TYPE_ELF);
> -      header->size = grub_cpu_to_be32 (mod_size + sizeof (*header));
> +      header->type = OBJ_TYPE_ELF;
> +      header->size = grub_host_to_target32 (mod_size + sizeof (*header));
>        offset += sizeof (*header);
>  
>        grub_util_load_image (p->name, kernel_img + offset);
> @@ -111,8 +111,8 @@ generate_image (const char *dir, const char *prefix, FILE 
> *out, char *mods[], ch
>        struct grub_module_header *header;
>  
>        header = (struct grub_module_header *) (kernel_img + offset);
> -      header->type = grub_cpu_to_be32 (OBJ_TYPE_MEMDISK);
> -      header->size = grub_cpu_to_be32 (memdisk_size + sizeof (*header));
> +      header->type = OBJ_TYPE_MEMDISK;
> +      header->size = grub_host_to_target32 (memdisk_size + sizeof (*header));
>        offset += sizeof (*header);
>  
>        grub_util_load_image (memdisk_path, kernel_img + offset);
> 
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
> 

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."