qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v2 1/5] define image_file_reset a


From: Alexander Graf
Subject: Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH v2 1/5] define image_file_reset and image_blob_reset
Date: Mon, 29 Oct 2012 09:41:42 +0100

Missing patch description

On 29.10.2012, at 06:21, Olivia Yin wrote:

> Signed-off-by: Olivia Yin <address@hidden>
> ---
> hw/loader.c |   39 +++++++++++++++++++++++++++++++++++++++
> hw/loader.h |   18 ++++++++++++++++++
> 2 files changed, 57 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/loader.c b/hw/loader.c
> index ba01ca6..cadf58f 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -86,6 +86,45 @@ int load_image(const char *filename, uint8_t *addr)
>     return size;
> }
> 

A comment describing the purpose of this function would be useful here.

> +void image_file_reset(void *opaque)
> +{
> +    ImageFile *image = opaque;
> +    GError *err = NULL;
> +    gboolean res;
> +    gchar *content;
> +    gsize size;
> +
> +    if(image->dir) {
> +        const char *basename;
> +        char fw_file_name[56];
> +
> +        basename = strrchr(image->name, '/');
> +        if (basename) {
> +            basename++;
> +        } else {
> +            basename = image->name;
> +        }

Do we have a helper for this that works on win32? Or do we always assume unix 
path names?

> +        snprintf(fw_file_name, sizeof(fw_file_name), "%s/%s", image->dir,
> +                 basename);
> +        image->name = g_strdup(fw_file_name);

Would asprintf(&image->name, ...) simplify this code? Same question about win32 
compatibility again.

> +    }
> +
> +    res = g_file_get_contents(image->name, &content, &size, &err);
> +    if (res == FALSE) {
> +       error_report("failed to read image file: %s\n", image->name);
> +       g_error_free(err);
> +    } else {
> +       cpu_physical_memory_rw(image->addr, (uint8_t *)content, size, 1);
> +       g_free(content);
> +    }
> +}
> +
> +void image_blob_reset(void *opaque)
> +{
> +    ImageBlob *blob = opaque;
> +    cpu_physical_memory_rw(blob->addr, blob->data, blob->size, 1);
> +}
> +
> /* read()-like version */
> ssize_t read_targphys(const char *name,
>                       int fd, hwaddr dst_addr, size_t nbytes)
> diff --git a/hw/loader.h b/hw/loader.h
> index 26480ad..e8030e7 100644
> --- a/hw/loader.h
> +++ b/hw/loader.h
> @@ -46,4 +46,22 @@ void do_info_roms(Monitor *mon);
> int rom_add_vga(const char *file);
> int rom_add_option(const char *file, int32_t bootindex);
> 
> +typedef struct ImageFile ImageFile;
> +struct ImageFile {
> +    char *name;
> +    char *dir;

In fact, can't this whole dir thing be handled on struct create time? What we 
basically need are 2 steps:

  1) Create struct with reset info. At this time we also want to sanity check 
all input and bail out on error.
  2) Load the image on reset. Here the error case is _very_ unlikely, unless 
someone just removed the file while we executed.


Alex

> +    hwaddr addr;
> +};
> +
> +typedef struct ImageBlob ImageBlob;
> +struct ImageBlob {
> +    char *name;
> +    hwaddr addr;
> +    ssize_t size;
> +    uint8_t *data;
> +};
> +
> +void image_blob_reset(void *opaque);
> +void image_file_reset(void *opaque);
> +
> #endif
> -- 
> 1.7.1
> 
> 
> 




reply via email to

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