Re: [PATCH 3/4] Change grub_file_seek() to return grub_err_t

[Vladimir 'phcoder' Serbinenko]

On Wed, Jul 22, 2009 at 5:16 AM, Pavel Roskin<address@hidden> wrote:
> No callers need the previous offset.  Returning -1 with implicit cast to
> grub_off_t required a cast just to check for errors.  This also makes
> grub_file_seek() more similar to fseek().
>
> ChangeLog:
>
>        * kern/file.c (grub_file_seek): Return grub_err_t.  Adjust all
>        callers that check the return value.
Good idea but you forgot to update the following entry:
./script/lua/grub_lib.c:270:  offset = grub_file_seek (file, offset);

> ---
>  commands/minicmd.c            |    4 ++--
>  font/font.c                   |    2 +-
>  include/grub/file.h           |    2 +-
>  kern/elf.c                    |   10 +++++-----
>  kern/file.c                   |   12 ++++--------
>  loader/aout.c                 |    2 +-
>  loader/i386/bsd.c             |    2 +-
>  loader/i386/bsdXX.c           |   10 +++++-----
>  loader/i386/multiboot.c       |    2 +-
>  loader/i386/multiboot_elfxx.c |    4 ++--
>  loader/macho.c                |   14 +++++++-------
>  loader/xnu_resume.c           |    2 +-
>  12 files changed, 31 insertions(+), 35 deletions(-)
>
> diff --git a/commands/minicmd.c b/commands/minicmd.c
> index 1f5abae..bc6458d 100644
> --- a/commands/minicmd.c
> +++ b/commands/minicmd.c
> @@ -188,7 +188,7 @@ grub_rescue_cmd_testload (int argc, char *argv[])
>
>   /* Read sequentially again.  */
>   grub_printf ("Reading %s sequentially again", argv[0]);
> -  if (grub_file_seek (file, 0) < 0)
> +  if (grub_file_seek (file, 0) != GRUB_ERR_NONE)
>     goto fail;
>
>   for (pos = 0; pos < size; pos += GRUB_DISK_SECTOR_SIZE)
> @@ -216,7 +216,7 @@ grub_rescue_cmd_testload (int argc, char *argv[])
>
>       pos -= GRUB_DISK_SECTOR_SIZE;
>
> -      if (grub_file_seek (file, pos) < 0)
> +      if (grub_file_seek (file, pos) != GRUB_ERR_NONE)
>        goto fail;
>
>       if (grub_file_read (file, sector, GRUB_DISK_SECTOR_SIZE)
> diff --git a/font/font.c b/font/font.c
> index a812919..67b97d2 100644
> --- a/font/font.c
> +++ b/font/font.c
> @@ -530,7 +530,7 @@ grub_font_load (const char *filename)
>           grub_printf("Unhandled section type, skipping.\n");
>  #endif
>           grub_off_t section_end = grub_file_tell (file) + section.length;
> -          if ((int) grub_file_seek (file, section_end) == -1)
> +          if (grub_file_seek (file, section_end) != GRUB_ERR_NONE)
>             goto fail;
>         }
>     }
> diff --git a/include/grub/file.h b/include/grub/file.h
> index 2aacf93..88fa088 100644
> --- a/include/grub/file.h
> +++ b/include/grub/file.h
> @@ -54,7 +54,7 @@ char *EXPORT_FUNC(grub_file_get_device_name) (const char 
> *name);
>  grub_file_t EXPORT_FUNC(grub_file_open) (const char *name);
>  grub_ssize_t EXPORT_FUNC(grub_file_read) (grub_file_t file, void *buf,
>                                          grub_size_t len);
> -grub_off_t EXPORT_FUNC(grub_file_seek) (grub_file_t file, grub_off_t offset);
> +grub_err_t EXPORT_FUNC(grub_file_seek) (grub_file_t file, grub_off_t offset);
>  grub_err_t EXPORT_FUNC(grub_file_close) (grub_file_t file);
>
>  static inline grub_off_t
> diff --git a/kern/elf.c b/kern/elf.c
> index f141610..fdb7d94 100644
> --- a/kern/elf.c
> +++ b/kern/elf.c
> @@ -67,7 +67,7 @@ grub_elf_file (grub_file_t file)
>
>   elf->file = file;
>
> -  if (grub_file_seek (elf->file, 0) == (grub_off_t) -1)
> +  if (grub_file_seek (elf->file, 0) != GRUB_ERR_NONE)
>     goto fail;
>
>   if (grub_file_read (elf->file, &elf->ehdr, sizeof (elf->ehdr))
> @@ -130,7 +130,7 @@ grub_elf32_load_phdrs (grub_elf_t elf)
>   if (! elf->phdrs)
>     return grub_errno;
>
> -  if ((grub_file_seek (elf->file, elf->ehdr.ehdr32.e_phoff) == (grub_off_t) 
> -1)
> +  if ((grub_file_seek (elf->file, elf->ehdr.ehdr32.e_phoff) != GRUB_ERR_NONE)
>       || (grub_file_read (elf->file, elf->phdrs, phdrs_size) != phdrs_size))
>     {
>       grub_error_push ();
> @@ -243,7 +243,7 @@ grub_elf32_load (grub_elf_t _elf, grub_elf32_load_hook_t 
> _load_hook,
>                  (unsigned long long) load_addr,
>                  (unsigned long long) phdr->p_memsz);
>
> -    if (grub_file_seek (elf->file, phdr->p_offset) == (grub_off_t) -1)
> +    if (grub_file_seek (elf->file, phdr->p_offset) != GRUB_ERR_NONE)
>       {
>        grub_error_push ();
>        return grub_error (GRUB_ERR_BAD_OS,
> @@ -309,7 +309,7 @@ grub_elf64_load_phdrs (grub_elf_t elf)
>   if (! elf->phdrs)
>     return grub_errno;
>
> -  if ((grub_file_seek (elf->file, elf->ehdr.ehdr64.e_phoff) == (grub_off_t) 
> -1)
> +  if ((grub_file_seek (elf->file, elf->ehdr.ehdr64.e_phoff) != GRUB_ERR_NONE)
>       || (grub_file_read (elf->file, elf->phdrs, phdrs_size) != phdrs_size))
>     {
>       grub_error_push ();
> @@ -423,7 +423,7 @@ grub_elf64_load (grub_elf_t _elf, grub_elf64_load_hook_t 
> _load_hook,
>                  (unsigned long long) load_addr,
>                  (unsigned long long) phdr->p_memsz);
>
> -    if (grub_file_seek (elf->file, phdr->p_offset) == (grub_off_t) -1)
> +    if (grub_file_seek (elf->file, phdr->p_offset) != GRUB_ERR_NONE)
>       {
>        grub_error_push ();
>        return grub_error (GRUB_ERR_BAD_OS,
> diff --git a/kern/file.c b/kern/file.c
> index 9b56b88..647fb8c 100644
> --- a/kern/file.c
> +++ b/kern/file.c
> @@ -141,19 +141,15 @@ grub_file_close (grub_file_t file)
>   return grub_errno;
>  }
>
> -grub_off_t
> +grub_err_t
>  grub_file_seek (grub_file_t file, grub_off_t offset)
>  {
> -  grub_off_t old;
> -
>   if (offset > file->size)
>     {
> -      grub_error (GRUB_ERR_OUT_OF_RANGE,
> -                 "attempt to seek outside of the file");
> -      return -1;
> +      return grub_error (GRUB_ERR_OUT_OF_RANGE,
> +                        "attempt to seek outside of the file");
>     }
>
> -  old = file->offset;
>   file->offset = offset;
> -  return old;
> +  return GRUB_ERR_NONE;
>  }
> diff --git a/loader/aout.c b/loader/aout.c
> index 0254b6a..a700771 100644
> --- a/loader/aout.c
> +++ b/loader/aout.c
> @@ -43,7 +43,7 @@ grub_aout_load (grub_file_t file, int offset,
>                int load_size,
>                 grub_addr_t bss_end_addr)
>  {
> -  if ((grub_file_seek (file, offset)) == (grub_off_t) - 1)
> +  if (grub_file_seek (file, offset) != GRUB_ERR_NONE)
>     return grub_errno;
>
>   if (!load_size)
> diff --git a/loader/i386/bsd.c b/loader/i386/bsd.c
> index 468e6d0..f592f81 100644
> --- a/loader/i386/bsd.c
> +++ b/loader/i386/bsd.c
> @@ -610,7 +610,7 @@ grub_bsd_load_aout (grub_file_t file)
>   int ofs, align_page;
>   union grub_aout_header ah;
>
> -  if ((grub_file_seek (file, 0)) == (grub_off_t) - 1)
> +  if ((grub_file_seek (file, 0)) != GRUB_ERR_NONE)
>     return grub_errno;
>
>   if (grub_file_read (file, &ah, sizeof (ah)) != sizeof (ah))
> diff --git a/loader/i386/bsdXX.c b/loader/i386/bsdXX.c
> index 3f15579..2ae542e 100644
> --- a/loader/i386/bsdXX.c
> +++ b/loader/i386/bsdXX.c
> @@ -13,7 +13,7 @@ load (grub_file_t file, void *where, grub_off_t off, 
> grub_size_t size)
>   if (PTR_TO_UINT32 (where) + size > grub_os_area_addr + grub_os_area_size)
>     return grub_error (GRUB_ERR_OUT_OF_RANGE,
>                       "Not enough memory for the module");
> -  if (grub_file_seek (file, off) == (grub_off_t) -1)
> +  if (grub_file_seek (file, off) != GRUB_ERR_NONE)
>     return grub_errno;
>   if (grub_file_read (file, where, size)
>       != (grub_ssize_t) size)
> @@ -29,7 +29,7 @@ load (grub_file_t file, void *where, grub_off_t off, 
> grub_size_t size)
>  static inline grub_err_t
>  read_headers (grub_file_t file, Elf_Ehdr *e, char **shdr)
>  {
> - if (grub_file_seek (file, 0) == (grub_off_t) -1)
> + if (grub_file_seek (file, 0) != GRUB_ERR_NONE)
>     return grub_errno;
>
>   if (grub_file_read (file, (char *) e, sizeof (*e)) != sizeof (*e))
> @@ -55,7 +55,7 @@ read_headers (grub_file_t file, Elf_Ehdr *e, char **shdr)
>   if (! *shdr)
>     return grub_errno;
>
> -  if (grub_file_seek (file, e->e_shoff) == (grub_off_t) -1)
> +  if (grub_file_seek (file, e->e_shoff) != GRUB_ERR_NONE)
>     return grub_errno;
>
>   if (grub_file_read (file, *shdr, e->e_shnum * e->e_shentsize)
> @@ -264,7 +264,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (grub_file_t file, 
> grub_addr_t *kern_end)
>   symstart = curload = ALIGN_UP (*kern_end, sizeof (grub_freebsd_addr_t));
>   *((grub_freebsd_addr_t *) UINT_TO_PTR (curload)) = symsize;
>   curload += sizeof (grub_freebsd_addr_t);
> -  if (grub_file_seek (file, symoff) == (grub_off_t) -1)
> +  if (grub_file_seek (file, symoff) != GRUB_ERR_NONE)
>     return grub_errno;
>   sym = (Elf_Sym *) UINT_TO_PTR (curload);
>   if (grub_file_read (file, UINT_TO_PTR (curload), symsize) !=
> @@ -278,7 +278,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (grub_file_t file, 
> grub_addr_t *kern_end)
>
>   *((grub_freebsd_addr_t *) UINT_TO_PTR (curload)) = strsize;
>   curload += sizeof (grub_freebsd_addr_t);
> -  if (grub_file_seek (file, stroff) == (grub_off_t) -1)
> +  if (grub_file_seek (file, stroff) != GRUB_ERR_NONE)
>     return grub_errno;
>   str = (char *) UINT_TO_PTR (curload);
>   if (grub_file_read (file, UINT_TO_PTR (curload), strsize)
> diff --git a/loader/i386/multiboot.c b/loader/i386/multiboot.c
> index 87ffcae..fc42cb1 100644
> --- a/loader/i386/multiboot.c
> +++ b/loader/i386/multiboot.c
> @@ -293,7 +293,7 @@ grub_multiboot (int argc, char *argv[])
>
>       grub_multiboot_payload_orig = (long) playground + 
> RELOCATOR_SIZEOF(forward);
>
> -      if ((grub_file_seek (file, offset)) == (grub_off_t) - 1)
> +      if ((grub_file_seek (file, offset)) != GRUB_ERR_NONE)
>        goto fail;
>
>       grub_file_read (file, (void *) grub_multiboot_payload_orig, load_size);
> diff --git a/loader/i386/multiboot_elfxx.c b/loader/i386/multiboot_elfxx.c
> index 77c4711..f1b7233 100644
> --- a/loader/i386/multiboot_elfxx.c
> +++ b/loader/i386/multiboot_elfxx.c
> @@ -115,8 +115,8 @@ CONCAT(grub_multiboot_load_elf, XX) (grub_file_t file, 
> void *buffer)
>          grub_dprintf ("multiboot_loader", "segment %d: paddr=0x%lx, 
> memsz=0x%lx, vaddr=0x%lx\n",
>                        i, (long) phdr(i)->p_paddr, (long) phdr(i)->p_memsz, 
> (long) phdr(i)->p_vaddr);
>
> -         if (grub_file_seek (file, (grub_off_t) phdr(i)->p_offset)
> -             == (grub_off_t) -1)
> +         if (grub_file_seek (file, (grub_off_t) phdr(i)->p_offset) !=
> +             GRUB_ERR_NONE)
>            return grub_error (GRUB_ERR_BAD_OS,
>                               "invalid offset in program header");
>
> diff --git a/loader/macho.c b/loader/macho.c
> index bd460b8..7fddaf0 100644
> --- a/loader/macho.c
> +++ b/loader/macho.c
> @@ -50,7 +50,7 @@ grub_macho_parse32 (grub_macho_t macho)
>     return;
>
>   /* Read header and check magic*/
> -  if (grub_file_seek (macho->file, macho->offset32) == (grub_off_t) -1
> +  if (grub_file_seek (macho->file, macho->offset32) != GRUB_ERR_NONE
>       || grub_file_read (macho->file, &head, sizeof (head))
>       != sizeof(head))
>     {
> @@ -123,7 +123,7 @@ grub_macho32_readfile (grub_macho_t macho, void *dest)
>     return grub_error (GRUB_ERR_BAD_OS,
>                       "Couldn't read architecture-specific part");
>
> -  if (grub_file_seek (macho->file, macho->offset32) == (grub_off_t) -1)
> +  if (grub_file_seek (macho->file, macho->offset32) != GRUB_ERR_NONE)
>     {
>       grub_error_push ();
>       return grub_error (GRUB_ERR_BAD_OS,
> @@ -206,8 +206,8 @@ grub_macho32_load (grub_macho_t macho, char *offset, int 
> flags)
>     if (! hdr->vmsize)
>       return 0;
>
> -    if (grub_file_seek (_macho->file, hdr->fileoff
> -                       + _macho->offset32) == (grub_off_t) -1)
> +    if (grub_file_seek (_macho->file, hdr->fileoff + _macho->offset32) !=
> +       GRUB_ERR_NONE)
>       {
>        grub_error_push ();
>        grub_error (GRUB_ERR_BAD_OS,
> @@ -297,7 +297,7 @@ grub_macho_file (grub_file_t file)
>   macho->cmds32 = 0;
>   macho->cmds64 = 0;
>
> -  if (grub_file_seek (macho->file, 0) == (grub_off_t) -1)
> +  if (grub_file_seek (macho->file, 0) != GRUB_ERR_NONE)
>     goto fail;
>
>   if (grub_file_read (macho->file, &filestart, sizeof (filestart))
> @@ -316,8 +316,8 @@ grub_macho_file (grub_file_t file)
>
>       /* Load architecture description. */
>       narchs = grub_be_to_cpu32 (filestart.fat.nfat_arch);
> -      if (grub_file_seek (macho->file, sizeof (struct grub_macho_fat_header))
> -         == (grub_off_t) -1)
> +      if (grub_file_seek (macho->file, sizeof (struct 
> grub_macho_fat_header)) !=
> +         GRUB_ERR_NONE)
>        goto fail;
>       archs = grub_malloc (sizeof (struct grub_macho_fat_arch) * narchs);
>       if (!archs)
> diff --git a/loader/xnu_resume.c b/loader/xnu_resume.c
> index 77f6887..998e61e 100644
> --- a/loader/xnu_resume.c
> +++ b/loader/xnu_resume.c
> @@ -103,7 +103,7 @@ grub_xnu_resume (char *imagename)
>     }
>
>   /* Read image. */
> -  if (grub_file_seek (file, 0) == (grub_off_t)-1
> +  if (grub_file_seek (file, 0) != GRUB_ERR_NONE
>       || grub_file_read (file, buf, hibhead.image_size)
>       != (grub_ssize_t) hibhead.image_size)
>     {
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git