[SeaBIOS] [PATCH v4 1/5] romfile_loader: utility to patch in-memory ROM files

[Kevin O'Connor]

On Mon, Sep 23, 2013 at 09:20:25PM +0300, Michael S. Tsirkin wrote:
> Add ability for a ROM file to point to
> it's image in memory. When file is in memory,
> add utility that can patch it, storing
> pointers to one file within another file.
> 
> This is not a lot of code: together with the follow-up patch to load
> ACPI tables from ROM, it's about 1K extra.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  Makefile             |   2 +-
>  src/romfile_loader.h |  72 +++++++++++++++++++++
>  src/fw/paravirt.c    |   2 +
>  src/romfile_loader.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 251 insertions(+), 1 deletion(-)
>  create mode 100644 src/romfile_loader.h
>  create mode 100644 src/romfile_loader.c
> 
> diff --git a/Makefile b/Makefile
> index 3984d35..ae36dba 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -23,7 +23,7 @@ SRC32FLAT=$(SRCBOTH) post.c memmap.c malloc.c pmm.c romfile.c optionroms.c \
>      hw/usb-hub.c \
>      fw/coreboot.c fw/lzmadecode.c fw/csm.c fw/biostables.c \
>      fw/paravirt.c fw/shadow.c fw/pciinit.c fw/smm.c fw/mtrr.c fw/xen.c \
> -    fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c
> +    fw/acpi.c fw/mptable.c fw/pirtable.c fw/smbios.c romfile_loader.c

romfile_loader is a firmware file and show go in the fw/ directory.

[...]
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -365,6 +365,8 @@ void qemu_cfg_init(void)
>      for (e = 0; e < count; e++) {
>          struct QemuCfgFile qfile;
>          qemu_cfg_read(&qfile, sizeof(qfile));
> +	if (!*qfile.name)
> +		return;

What is this for?

SeaBIOS uses spaces and no tabs.

[...]
> +int romfile_loader_execute(const char *name)
> +{
> +    struct romfile_loader_entry_s *entry;
> +    int size, offset = 0, nfiles = 0;
> +    struct romfile_loader_files *files;
> +    void *data = romfile_loadfile(name, &size);
> +    if (!data)
> +        return -1;
> +
> +    /* Validate and count files */
> +    for (offset = 0; offset < size; offset += sizeof(*entry)) {
> +        entry = data + offset;
> +        /* Check that entry fits in buffer. */
> +        if (offset + sizeof(*entry) > size) {
> +            warn_internalerror();
> +            return -1;
> +        }
> +
> +        if (le32_to_cpu(entry->command) == ROMFILE_LOADER_COMMAND_ALLOCATE) {
> +            nfiles++;
> +	}
> +    }
> +
> +    files = malloc_tmp(sizeof(*files) + nfiles * sizeof(files->files[0]));
> +    files->nfiles = 0;

Nitpick, but this would be much simpler if it was just:
files = malloc_tmp(size / sizeof(*entry) * sizeof(*files));
Also, you should check that that the malloc succeeds.

Otherwise, it looks okay to me.

-Kevin