On 01/22/2014 04:20 PM, Alexey Kardashevskiy wrote: > The existing load_elf() just returns -1 if it fails to load ELF. However > it could be smarter than this and tell more about the failure such as > wrong endianness or incompatible platform. > > This adds additional return codes for wrong architecture, wrong > endianness and if the image is not ELF at all. > > This fixes handling of what load_elf() returns for s390x and moxie > architectures, other callers just check the return value for <0 and > this remains unchanged. > > Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru> > --- > hw/core/loader.c | 12 ++++++------ > hw/s390x/ipl.c | 4 ++-- > include/hw/elf_ops.h | 19 ++++++++++++++----- > include/hw/loader.h | 5 +++++ > 4 files changed, 27 insertions(+), 13 deletions(-) > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 0634bee..f510260 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -289,7 +289,7 @@ int load_elf(const char *filename, uint64_t > (*translate_fn)(void *, uint64_t), > void *translate_opaque, uint64_t *pentry, uint64_t *lowaddr, > uint64_t *highaddr, int big_endian, int elf_machine, int > clear_lsb) > { > - int fd, data_order, target_data_order, must_swab, ret; > + int fd, data_order, target_data_order, must_swab, ret = ELF_LOAD_FAILED; > uint8_t e_ident[EI_NIDENT]; > > fd = open(filename, O_RDONLY | O_BINARY); > @@ -302,8 +302,10 @@ int load_elf(const char *filename, uint64_t > (*translate_fn)(void *, uint64_t), > if (e_ident[0] != ELFMAG0 || > e_ident[1] != ELFMAG1 || > e_ident[2] != ELFMAG2 || > - e_ident[3] != ELFMAG3) > + e_ident[3] != ELFMAG3) { > + ret = ELF_LOAD_NOT_ELF; > goto fail; > + } > #ifdef HOST_WORDS_BIGENDIAN > data_order = ELFDATA2MSB; > #else > @@ -317,6 +319,7 @@ int load_elf(const char *filename, uint64_t > (*translate_fn)(void *, uint64_t), > } > > if (target_data_order != e_ident[EI_DATA]) { > + ret = ELF_LOAD_WRONG_ENDIAN; > goto fail; > } > > @@ -329,12 +332,9 @@ int load_elf(const char *filename, uint64_t > (*translate_fn)(void *, uint64_t), > pentry, lowaddr, highaddr, elf_machine, clear_lsb); > } > > - close(fd); > - return ret; > - > fail: > close(fd); > - return -1; > + return ret; > } > > static void bswap_uboot_header(uboot_image_header_t *hdr) > diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c > index 1a6397b..cff77ad 100644 > --- a/hw/s390x/ipl.c > +++ b/hw/s390x/ipl.c > @@ -97,10 +97,10 @@ static int s390_ipl_init(SysBusDevice *dev) > } else { > kernel_size = load_elf(ipl->kernel, NULL, NULL, NULL, NULL, > NULL, 1, ELF_MACHINE, 0); > - if (kernel_size == -1) { > + if (kernel_size < 0) { > kernel_size = load_image_targphys(ipl->kernel, 0, ram_size); > } > - if (kernel_size == -1) { > + if (kernel_size < 0) { > fprintf(stderr, "could not load kernel '%s'\n", ipl->kernel); > return -1; > } > diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h > index acc701e..b7e7b36 100644 > --- a/include/hw/elf_ops.h > +++ b/include/hw/elf_ops.h > @@ -201,6 +201,7 @@ static int glue(load_elf, SZ)(const char *name, int fd, > uint64_t addr, low = (uint64_t)-1, high = 0; > uint8_t *data = NULL; > char label[128]; > + int ret = ELF_LOAD_FAILED; > > if (read(fd, &ehdr, sizeof(ehdr)) != sizeof(ehdr)) > goto fail; > @@ -210,23 +211,31 @@ static int glue(load_elf, SZ)(const char *name, int fd, > > switch (elf_machine) { > case EM_PPC64: > - if (EM_PPC64 != ehdr.e_machine) > + if (EM_PPC64 != ehdr.e_machine) { > if (EM_PPC != ehdr.e_machine)
A stupid bug here, "{" should go one line down :) I'll repost if someone tells me that the whole idea makes any sense. Thanks. -- Alexey