[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] Allow larger return values from get_image_s
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] Allow larger return values from get_image_size() |
Date: |
Fri, 24 Feb 2012 02:48:46 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Feb 24, 2012 at 11:36:45AM +1100, David Gibson wrote:
> Currently get_image_size(), used to find the size of files, returns an int.
> But for modern systems, int may be only 32-bit and we can have files
> larger than that.
>
> This patch, therefore, changes the return type of get_image_size() to off_t
> (the same as the return type from lseek() itself). It also audits all the
> callers of get_image_size() to make sure they process the new unsigned
> return type correctly.
>
> This leaves load_image_targphys() with a limited return type, but one thing
> at a time (that function has far more callers to be audited, so it will
> take longer to fix).
>
> Signed-off-by: David Gibson <address@hidden>
Hmm, this can still be 32 bit.
We probably should set
#define_FILE_OFFSET_BITS 64
before including any standard headers.
This will make it 64 bit consistently.
> ---
> device_tree.c | 4 ++--
> hw/alpha_dp264.c | 5 +++--
> hw/highbank.c | 2 +-
> hw/leon3.c | 6 +++---
> hw/loader.c | 12 +++++++-----
> hw/loader.h | 2 +-
> hw/mips_fulong2e.c | 12 ++++++------
> hw/mips_malta.c | 12 ++++++------
> hw/mips_mipssim.c | 6 +++---
> hw/mips_r4k.c | 17 +++++++++--------
> hw/multiboot.c | 4 ++--
> hw/palm.c | 11 ++++++-----
> hw/pc.c | 5 +++--
> hw/pc_sysfw.c | 6 +++---
> hw/pci.c | 4 ++--
> hw/ppc_prep.c | 7 ++++---
> hw/smbios.c | 2 +-
> 17 files changed, 62 insertions(+), 55 deletions(-)
>
> diff --git a/device_tree.c b/device_tree.c
> index 86a694c..5817b2d 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -27,14 +27,14 @@
>
> void *load_device_tree(const char *filename_path, int *sizep)
> {
> - int dt_size;
> + off_t dt_size;
> int dt_file_load_size;
> int ret;
> void *fdt = NULL;
>
> *sizep = 0;
> dt_size = get_image_size(filename_path);
> - if (dt_size < 0) {
> + if (dt_size == -1) {
> printf("Unable to get size of device tree file '%s'\n",
> filename_path);
> goto fail;
> diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c
> index ea0fd95..01ec9c7 100644
> --- a/hw/alpha_dp264.c
> +++ b/hw/alpha_dp264.c
> @@ -144,10 +144,11 @@ static void clipper_init(ram_addr_t ram_size,
> }
>
> if (initrd_filename) {
> - long initrd_base, initrd_size;
> + long initrd_base;
> + off_t initrd_size;
>
> initrd_size = get_image_size(initrd_filename);
> - if (initrd_size < 0) {
> + if (initrd_size != -1) {
> hw_error("could not load initial ram disk '%s'\n",
> initrd_filename);
> exit(1);
> diff --git a/hw/highbank.c b/hw/highbank.c
> index 489c00e..f4de4c6 100644
> --- a/hw/highbank.c
> +++ b/hw/highbank.c
> @@ -235,7 +235,7 @@ static void highbank_init(ram_addr_t ram_size,
> if (bios_name != NULL) {
> sysboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> if (sysboot_filename != NULL) {
> - uint32_t filesize = get_image_size(sysboot_filename);
> + off_t filesize = get_image_size(sysboot_filename);
> if (load_image_targphys("sysram.bin", 0xfff88000, filesize) < 0)
> {
> hw_error("Unable to load %s\n", bios_name);
> }
> diff --git a/hw/leon3.c b/hw/leon3.c
> index 71d79a6..f7d4860 100644
> --- a/hw/leon3.c
> +++ b/hw/leon3.c
> @@ -108,7 +108,7 @@ static void leon3_generic_hw_init(ram_addr_t ram_size,
> int ret;
> char *filename;
> qemu_irq *cpu_irqs = NULL;
> - int bios_size;
> + off_t bios_size;
> int prom_size;
> ResetData *reset_info;
>
> @@ -168,7 +168,7 @@ static void leon3_generic_hw_init(ram_addr_t ram_size,
> exit(1);
> }
>
> - if (bios_size > 0) {
> + if (bios_size != -1) {
> ret = load_image_targphys(filename, 0x00000000, bios_size);
> if (ret < 0 || ret > prom_size) {
> fprintf(stderr, "qemu: could not load prom '%s'\n", filename);
> @@ -191,7 +191,7 @@ static void leon3_generic_hw_init(ram_addr_t ram_size,
> kernel_filename);
> exit(1);
> }
> - if (bios_size <= 0) {
> + if (bios_size == 0 || bios_size == -1) {
> /* If there is no bios/monitor, start the application. */
> env->pc = entry;
> env->npc = entry + 4;
> diff --git a/hw/loader.c b/hw/loader.c
> index 415cdce..372f6f8 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -57,12 +57,14 @@
> static int roms_loaded;
>
> /* return the size or -1 if error */
> -int get_image_size(const char *filename)
> +off_t get_image_size(const char *filename)
> {
> - int fd, size;
> + int fd;
> + off_t size;
> +
> fd = open(filename, O_RDONLY | O_BINARY);
> if (fd < 0)
> - return -1;
> + return (off_t)-1;
> size = lseek(fd, 0, SEEK_END);
> close(fd);
> return size;
> @@ -105,13 +107,13 @@ ssize_t read_targphys(const char *name,
> int load_image_targphys(const char *filename,
> target_phys_addr_t addr, int max_sz)
> {
> - int size;
> + off_t size;
>
> size = get_image_size(filename);
> if (size > max_sz) {
> return -1;
> }
> - if (size > 0) {
> + if (size != -1) {
> rom_add_file_fixed(filename, addr, -1);
> }
> return size;
> diff --git a/hw/loader.h b/hw/loader.h
> index fbcaba9..1fbdbcf 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -2,7 +2,7 @@
> #define LOADER_H
>
> /* loader.c */
> -int get_image_size(const char *filename);
> +off_t get_image_size(const char *filename);
> int load_image(const char *filename, uint8_t *addr); /* deprecated */
> int load_image_targphys(const char *filename, target_phys_addr_t, int
> max_sz);
> int load_elf(const char *filename, uint64_t (*translate_fn)(void *,
> uint64_t),
> diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c
> index e3ba9dd..1e7b49a 100644
> --- a/hw/mips_fulong2e.c
> +++ b/hw/mips_fulong2e.c
> @@ -107,7 +107,7 @@ static int64_t load_kernel (CPUState *env)
> {
> int64_t kernel_entry, kernel_low, kernel_high;
> int index = 0;
> - long initrd_size;
> + off_t initrd_size;
> ram_addr_t initrd_offset;
> uint32_t *prom_buf;
> long prom_size;
> @@ -125,7 +125,7 @@ static int64_t load_kernel (CPUState *env)
> initrd_offset = 0;
> if (loaderparams.initrd_filename) {
> initrd_size = get_image_size (loaderparams.initrd_filename);
> - if (initrd_size > 0) {
> + if (initrd_size != -1) {
> initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) &
> TARGET_PAGE_MASK;
> if (initrd_offset + initrd_size > ram_size) {
> fprintf(stderr,
> @@ -136,7 +136,7 @@ static int64_t load_kernel (CPUState *env)
> initrd_size = load_image_targphys(loaderparams.initrd_filename,
> initrd_offset, ram_size -
> initrd_offset);
> }
> - if (initrd_size == (target_ulong) -1) {
> + if (initrd_size == -1) {
> fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
> loaderparams.initrd_filename);
> exit(1);
> @@ -148,10 +148,10 @@ static int64_t load_kernel (CPUState *env)
> prom_buf = g_malloc(prom_size);
>
> prom_set(prom_buf, index++, "%s", loaderparams.kernel_filename);
> - if (initrd_size > 0) {
> + if (initrd_size != -1) {
> prom_set(prom_buf, index++, "rd_start=0x%" PRIx64 " rd_size=%li %s",
> - cpu_mips_phys_to_kseg0(NULL, initrd_offset), initrd_size,
> - loaderparams.kernel_cmdline);
> + cpu_mips_phys_to_kseg0(NULL, initrd_offset),
> + (long)initrd_size, loaderparams.kernel_cmdline);
> } else {
> prom_set(prom_buf, index++, "%s", loaderparams.kernel_cmdline);
> }
> diff --git a/hw/mips_malta.c b/hw/mips_malta.c
> index 86a5fbb..50e800a 100644
> --- a/hw/mips_malta.c
> +++ b/hw/mips_malta.c
> @@ -667,7 +667,7 @@ static void GCC_FMT_ATTR(3, 4) prom_set(uint32_t*
> prom_buf, int index,
> static int64_t load_kernel (void)
> {
> int64_t kernel_entry, kernel_high;
> - long initrd_size;
> + off_t initrd_size;
> ram_addr_t initrd_offset;
> int big_endian;
> uint32_t *prom_buf;
> @@ -693,7 +693,7 @@ static int64_t load_kernel (void)
> initrd_offset = 0;
> if (loaderparams.initrd_filename) {
> initrd_size = get_image_size (loaderparams.initrd_filename);
> - if (initrd_size > 0) {
> + if (initrd_size != -1) {
> initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) &
> TARGET_PAGE_MASK;
> if (initrd_offset + initrd_size > ram_size) {
> fprintf(stderr,
> @@ -705,7 +705,7 @@ static int64_t load_kernel (void)
> initrd_offset,
> ram_size - initrd_offset);
> }
> - if (initrd_size == (target_ulong) -1) {
> + if (initrd_size == -1) {
> fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
> loaderparams.initrd_filename);
> exit(1);
> @@ -717,10 +717,10 @@ static int64_t load_kernel (void)
> prom_buf = g_malloc(prom_size);
>
> prom_set(prom_buf, prom_index++, "%s", loaderparams.kernel_filename);
> - if (initrd_size > 0) {
> + if (initrd_size != -1) {
> prom_set(prom_buf, prom_index++, "rd_start=0x%" PRIx64 " rd_size=%li
> %s",
> - cpu_mips_phys_to_kseg0(NULL, initrd_offset), initrd_size,
> - loaderparams.kernel_cmdline);
> + cpu_mips_phys_to_kseg0(NULL, initrd_offset),
> + (long)initrd_size, loaderparams.kernel_cmdline);
> } else {
> prom_set(prom_buf, prom_index++, "%s", loaderparams.kernel_cmdline);
> }
> diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c
> index 76c95b2..89de6ca 100644
> --- a/hw/mips_mipssim.c
> +++ b/hw/mips_mipssim.c
> @@ -54,7 +54,7 @@ static int64_t load_kernel(void)
> {
> int64_t entry, kernel_high;
> long kernel_size;
> - long initrd_size;
> + off_t initrd_size;
> ram_addr_t initrd_offset;
> int big_endian;
>
> @@ -82,7 +82,7 @@ static int64_t load_kernel(void)
> initrd_offset = 0;
> if (loaderparams.initrd_filename) {
> initrd_size = get_image_size (loaderparams.initrd_filename);
> - if (initrd_size > 0) {
> + if (initrd_size != -1) {
> initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) &
> TARGET_PAGE_MASK;
> if (initrd_offset + initrd_size > loaderparams.ram_size) {
> fprintf(stderr,
> @@ -93,7 +93,7 @@ static int64_t load_kernel(void)
> initrd_size = load_image_targphys(loaderparams.initrd_filename,
> initrd_offset, loaderparams.ram_size - initrd_offset);
> }
> - if (initrd_size == (target_ulong) -1) {
> + if (initrd_size == -1) {
> fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
> loaderparams.initrd_filename);
> exit(1);
> diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
> index 83401f0..067a793 100644
> --- a/hw/mips_r4k.c
> +++ b/hw/mips_r4k.c
> @@ -72,7 +72,8 @@ typedef struct ResetData {
> static int64_t load_kernel(void)
> {
> int64_t entry, kernel_high;
> - long kernel_size, initrd_size, params_size;
> + long kernel_size, params_size;
> + off_t initrd_size;
> ram_addr_t initrd_offset;
> uint32_t *params_buf;
> int big_endian;
> @@ -100,7 +101,7 @@ static int64_t load_kernel(void)
> initrd_offset = 0;
> if (loaderparams.initrd_filename) {
> initrd_size = get_image_size (loaderparams.initrd_filename);
> - if (initrd_size > 0) {
> + if (initrd_size != -1) {
> initrd_offset = (kernel_high + ~TARGET_PAGE_MASK) &
> TARGET_PAGE_MASK;
> if (initrd_offset + initrd_size > ram_size) {
> fprintf(stderr,
> @@ -112,7 +113,7 @@ static int64_t load_kernel(void)
> initrd_offset,
> ram_size - initrd_offset);
> }
> - if (initrd_size == (target_ulong) -1) {
> + if (initrd_size == -1) {
> fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
> loaderparams.initrd_filename);
> exit(1);
> @@ -126,10 +127,10 @@ static int64_t load_kernel(void)
> params_buf[0] = tswap32(ram_size);
> params_buf[1] = tswap32(0x12345678);
>
> - if (initrd_size > 0) {
> + if (initrd_size != -1) {
> snprintf((char *)params_buf + 8, 256, "rd_start=0x%" PRIx64 "
> rd_size=%li %s",
> cpu_mips_phys_to_kseg0(NULL, initrd_offset),
> - initrd_size, loaderparams.kernel_cmdline);
> + (long)initrd_size, loaderparams.kernel_cmdline);
> } else {
> snprintf((char *)params_buf + 8, 256, "%s",
> loaderparams.kernel_cmdline);
> }
> @@ -161,7 +162,7 @@ void mips_r4k_init (ram_addr_t ram_size,
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> MemoryRegion *bios;
> MemoryRegion *iomem = g_new(MemoryRegion, 1);
> - int bios_size;
> + off_t bios_size;
> CPUState *env;
> ResetData *reset_info;
> int i;
> @@ -214,14 +215,14 @@ void mips_r4k_init (ram_addr_t ram_size,
> if (filename) {
> bios_size = get_image_size(filename);
> } else {
> - bios_size = -1;
> + bios_size = (off_t)-1;
> }
> #ifdef TARGET_WORDS_BIGENDIAN
> be = 1;
> #else
> be = 0;
> #endif
> - if ((bios_size > 0) && (bios_size <= BIOS_SIZE)) {
> + if ((bios_size != -1) && (bios_size <= BIOS_SIZE)) {
> bios = g_new(MemoryRegion, 1);
> memory_region_init_ram(bios, "mips_r4k.bios", BIOS_SIZE);
> vmstate_register_ram_global(bios);
> diff --git a/hw/multiboot.c b/hw/multiboot.c
> index b4484a3..37e8af5 100644
> --- a/hw/multiboot.c
> +++ b/hw/multiboot.c
> @@ -262,7 +262,7 @@ int load_multiboot(void *fw_cfg,
>
> do {
> char *next_space;
> - int mb_mod_length;
> + off_t mb_mod_length;
> uint32_t offs = mbs.mb_buf_size;
>
> next_initrd = (char *)get_opt_value(NULL, 0, initrd_filename);
> @@ -275,7 +275,7 @@ int load_multiboot(void *fw_cfg,
> *next_space = '\0';
> mb_debug("multiboot loading module: %s\n", initrd_filename);
> mb_mod_length = get_image_size(initrd_filename);
> - if (mb_mod_length < 0) {
> + if (mb_mod_length == -1) {
> fprintf(stderr, "Failed to open file '%s'\n",
> initrd_filename);
> exit(1);
> }
> diff --git a/hw/palm.c b/hw/palm.c
> index b1252ab..176ad4b 100644
> --- a/hw/palm.c
> +++ b/hw/palm.c
> @@ -203,7 +203,8 @@ static void palmte_init(ram_addr_t ram_size,
> static uint32_t cs1val = 0x0000e1a0;
> static uint32_t cs2val = 0x0000e1a0;
> static uint32_t cs3val = 0xe1a0e1a0;
> - int rom_size, rom_loaded = 0;
> + int rom_loaded = 0;
> + off_t rom_size;
> DisplayState *ds = get_displaystate();
> MemoryRegion *flash = g_new(MemoryRegion, 1);
> MemoryRegion *cs = g_new(MemoryRegion, 4);
> @@ -240,16 +241,16 @@ static void palmte_init(ram_addr_t ram_size,
> if (nb_option_roms) {
> rom_size = get_image_size(option_rom[0].name);
> if (rom_size > flash_size) {
> - fprintf(stderr, "%s: ROM image too big (%x > %x)\n",
> - __FUNCTION__, rom_size, flash_size);
> + fprintf(stderr, "%s: ROM image too big (%lx > %x)\n",
> + __FUNCTION__, (unsigned long)rom_size, flash_size);
> rom_size = 0;
> }
> - if (rom_size > 0) {
> + if (rom_size != -1) {
> rom_size = load_image_targphys(option_rom[0].name, OMAP_CS0_BASE,
> flash_size);
> rom_loaded = 1;
> }
> - if (rom_size < 0) {
> + if (rom_size == -1) {
> fprintf(stderr, "%s: error loading '%s'\n",
> __FUNCTION__, option_rom[0].name);
> }
> diff --git a/hw/pc.c b/hw/pc.c
> index b9f4bc7..cb41955 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -672,7 +672,8 @@ static void load_linux(void *fw_cfg,
> target_phys_addr_t max_ram_size)
> {
> uint16_t protocol;
> - int setup_size, kernel_size, initrd_size = 0, cmdline_size;
> + int setup_size, kernel_size, cmdline_size;
> + off_t initrd_size = 0;
> uint32_t initrd_max;
> uint8_t header[8192], *setup, *kernel, *initrd_data;
> target_phys_addr_t real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
> @@ -795,7 +796,7 @@ static void load_linux(void *fw_cfg,
> }
>
> initrd_size = get_image_size(initrd_filename);
> - if (initrd_size < 0) {
> + if (initrd_size == -1) {
> fprintf(stderr, "qemu: error reading initrd %s\n",
> initrd_filename);
> exit(1);
> diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
> index abf9004..7e2f715 100644
> --- a/hw/pc_sysfw.c
> +++ b/hw/pc_sysfw.c
> @@ -132,7 +132,7 @@ static void old_pc_system_rom_init(MemoryRegion
> *rom_memory)
> {
> char *filename;
> MemoryRegion *bios, *isa_bios;
> - int bios_size, isa_bios_size;
> + off_t bios_size, isa_bios_size;
> int ret;
>
> /* BIOS load */
> @@ -143,9 +143,9 @@ static void old_pc_system_rom_init(MemoryRegion
> *rom_memory)
> if (filename) {
> bios_size = get_image_size(filename);
> } else {
> - bios_size = -1;
> + bios_size = (off_t)-1;
> }
> - if (bios_size <= 0 ||
> + if ((bios_size == 0) || (bios_size == -1) ||
> (bios_size % 65536) != 0) {
> goto bios_error;
> }
> diff --git a/hw/pci.c b/hw/pci.c
> index bf046bf..5720a8c 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1672,7 +1672,7 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t
> *ptr, int size)
> /* Add an option rom for the device */
> static int pci_add_option_rom(PCIDevice *pdev, bool is_default_rom)
> {
> - int size;
> + off_t size;
> char *path;
> void *ptr;
> char name[32];
> @@ -1703,7 +1703,7 @@ static int pci_add_option_rom(PCIDevice *pdev, bool
> is_default_rom)
> }
>
> size = get_image_size(path);
> - if (size < 0) {
> + if (size == -1) {
> error_report("%s: failed to find romfile \"%s\"",
> __FUNCTION__, pdev->romfile);
> g_free(path);
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index eb43fb5..c41e13c 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -489,7 +489,8 @@ static void ppc_prep_init (ram_addr_t ram_size,
> #if 0
> MemoryRegion *xcsr = g_new(MemoryRegion, 1);
> #endif
> - int linux_boot, i, nb_nics1, bios_size;
> + int linux_boot, i, nb_nics1;
> + off_t bios_size;
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> MemoryRegion *bios = g_new(MemoryRegion, 1);
> uint32_t kernel_base, initrd_base;
> @@ -546,13 +547,13 @@ static void ppc_prep_init (ram_addr_t ram_size,
> } else {
> bios_size = -1;
> }
> - if (bios_size > 0 && bios_size <= BIOS_SIZE) {
> + if (bios_size != -1 && bios_size <= BIOS_SIZE) {
> target_phys_addr_t bios_addr;
> bios_size = (bios_size + 0xfff) & ~0xfff;
> bios_addr = (uint32_t)(-bios_size);
> bios_size = load_image_targphys(filename, bios_addr, bios_size);
> }
> - if (bios_size < 0 || bios_size > BIOS_SIZE) {
> + if (bios_size == -1 || bios_size > BIOS_SIZE) {
> hw_error("qemu: could not load PPC PREP bios '%s'\n", bios_name);
> }
> if (filename) {
> diff --git a/hw/smbios.c b/hw/smbios.c
> index c57237d..85fad0e 100644
> --- a/hw/smbios.c
> +++ b/hw/smbios.c
> @@ -185,7 +185,7 @@ int smbios_entry_add(const char *t)
> if (get_param_value(buf, sizeof(buf), "file", t)) {
> struct smbios_structure_header *header;
> struct smbios_table *table;
> - int size = get_image_size(buf);
> + off_t size = get_image_size(buf);
>
> if (size == -1 || size < sizeof(struct smbios_structure_header)) {
> fprintf(stderr, "Cannot read smbios file %s\n", buf);
> --
> 1.7.9
>