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

[Michael S. Tsirkin]

On Tue, Sep 24, 2013 at 08:16:47PM -0400, Kevin O'Connor wrote:
> On Wed, Sep 25, 2013 at 03:11:20AM +0300, Michael S. Tsirkin wrote:
> > On Tue, Sep 24, 2013 at 07:55:52PM -0400, Kevin O'Connor wrote:
> > > In all real-world situations we'll have megabytes of ram for "tmp"
> > > allocations.  It's not a problem to over allocate here.
> > 
> > Right. But the code doesn't get simpler in fact - unless you
> > think it's important to drop the loop counting nfiles?
> 
> I was pointing out the loop, not the malloc call.  As before, it's a
> nitpick.
> 
> -Kevin

Ah, I see what you mean then. something like this on top
will do the trick. Yes, it's simpler.


diff --git a/src/fw/romfile_loader.c b/src/fw/romfile_loader.c
index c03fe48..2196a84 100644
--- a/src/fw/romfile_loader.c
+++ b/src/fw/romfile_loader.c
@@ -130,30 +130,23 @@ err:
 int romfile_loader_execute(const char *name)
 {
     struct romfile_loader_entry_s *entry;
-    int size, offset = 0, nfiles = 0;
+    int size, offset = 0, nfiles;
     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++;
-        }
+    if (size % sizeof(*entry)) {
+        warn_invalid();
+        goto err;
     }
 
+    /* (over)estimate the number of files to load. */
+    nfiles = size / sizeof(*entry);
     files = malloc_tmp(sizeof(*files) + nfiles * sizeof(files->files[0]));
     if (!files) {
         warn_noalloc();
-        return -1;
+        goto err;
     }
     files->nfiles = 0;
 
@@ -177,4 +170,8 @@ int romfile_loader_execute(const char *name)
     free(files);
     free(data);
     return 0;
+
+err:
+    free(data);
+    return -1;
 }