[coreboot-gerrit] Change in ...coreboot[master]: util/amdfwtool: Add generic image copy function

[Martin Roth (Code Review)]

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31733 )

Change subject: util/amdfwtool: Add generic image copy function
......................................................................

util/amdfwtool: Add generic image copy function

Consolidate the code that opens, stats, copies, and closes the
individual files into a single function.

TEST=Verify no difference in amdfw.rom for google/grunt before
     and after the patch is applied

Change-Id: I2da0dd79186ccc8c762b58cf3decb9980378a5f7
Signed-off-by: Marshall Dawson 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31733
Tested-by: build bot (Jenkins) 
Reviewed-by: Martin Roth 
---
M util/amdfwtool/amdfwtool.c
1 file changed, 46 insertions(+), 68 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Martin Roth: Looks good to me, approved

diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c
index 774eefe..3303a57 100644
--- a/util/amdfwtool/amdfwtool.c
+++ b/util/amdfwtool/amdfwtool.c
@@ -322,30 +322,48 @@
    }
 }

+static ssize_t copy_blob(void *dest, const char *src_file, size_t room)
+{
+	int fd;
+	struct stat fd_stat;
+	ssize_t bytes;
+
+	fd = open(src_file, O_RDONLY);
+	if (fd < 0) {
+		printf("Error: %s\n", strerror(errno));
+		return -1;
+	}
+
+	if (fstat(fd, &fd_stat)) {
+		printf("fstat error: %s\n", strerror(errno));
+		return -2;
+	}
+
+	if (fd_stat.st_size > room) {
+		printf("Error: %s will not fit.  Exiting.\n", src_file);
+		return -3;
+	}
+
+	bytes = read(fd, dest, (size_t)fd_stat.st_size);
+	close(fd);
+	if (bytes != (ssize_t)fd_stat.st_size) {
+		printf("Error while reading %s\n", src_file);
+		return -4;
+	}
+
+	return bytes;
+}
+
 static uint32_t integrate_firmwares(char *base, uint32_t pos,
    			embedded_firmware *romsig,
    			amd_fw_entry *fw_table, uint32_t rom_size)
 {
-	int fd;
    ssize_t bytes;
-	struct stat fd_stat;
    int i;
    uint32_t rom_base_address = 0xFFFFFFFF - rom_size + 1;

for (i = 0; fw_table[i].type != AMD_FW_INVALID; i++) {
    	if (fw_table[i].filename != NULL) {
-			fd = open(fw_table[i].filename, O_RDONLY);
-			if (fd < 0) {
-				printf("Error: %s\n", strerror(errno));
-				free(base);
-				exit(1);
-			}
-			if (fstat(fd, &fd_stat)) {
-				printf("fstat error: %s\n", strerror(errno));
-				free(base);
-				exit(1);
-			}
-
    		switch (fw_table[i].type) {
    		case AMD_FW_IMC:
    			pos = ALIGN(pos, 0x10000U);
@@ -362,27 +380,14 @@
    			break;
    		}

-			if (pos + fd_stat.st_size > rom_size) {
-				printf("Error: Specified ROM size of %d"
-					" will not fit %s.  Exiting.\n",
-					rom_size, fw_table[i].filename);
+			bytes = copy_blob(base + pos,
+					fw_table[i].filename, rom_size - pos);
+			if (bytes <= 0) {
    			free(base);
    			exit(1);
    		}

-			bytes = read(fd, (void *)(base + pos),
-					(size_t)fd_stat.st_size);
-			if (bytes == (ssize_t)fd_stat.st_size)
-				pos += fd_stat.st_size;
-			else {
-				printf("Error while reading %s\n",
-					fw_table[i].filename);
-				free(base);
-				exit(1);
-			}
-
-			close(fd);
-			pos = ALIGN(pos, 0x100U);
+			pos = ALIGN(pos + bytes, 0x100U);
    	}
    }

@@ -394,9 +399,7 @@
    				amd_fw_entry *fw_table,
    				uint32_t rom_size)
 {
-	int fd;
    ssize_t bytes;
-	struct stat fd_stat;
    unsigned int i, count;
    uint32_t rom_base_address = 0xFFFFFFFF - rom_size + 1;

@@ -407,43 +410,18 @@
    		pspdir->entries[count].addr = 1;
    		count++;
    	} else if (fw_table[i].filename != NULL) {
+			bytes = copy_blob(base + pos,
+					fw_table[i].filename, rom_size - pos);
+			if (bytes <= 0) {
+				free(base);
+				exit(1);
+			}
+
    		pspdir->entries[count].type = fw_table[i].type;
+			pspdir->entries[count].size = (uint32_t)bytes;
+			pspdir->entries[count].addr = rom_base_address + pos;

-			fd = open(fw_table[i].filename, O_RDONLY);
-			if (fd < 0) {
-				printf("Error: %s\n", strerror(errno));
-				free(base);
-				exit(1);
-			}
-			if (fstat(fd, &fd_stat)) {
-				printf("fstat error: %s\n", strerror(errno));
-				free(base);
-				exit(1);
-			}
-			pspdir->entries[count].size = (uint32_t)fd_stat.st_size;
-			pspdir->entries[count].addr = pos + rom_base_address;
-
-			if (pos + fd_stat.st_size > rom_size) {
-				printf("Error: Specified ROM size of %d"
-					" will not fit %s.  Exiting.\n",
-					rom_size, fw_table[i].filename);
-				free(base);
-				exit(1);
-			}
-
-			bytes = read(fd, (void *)(base + pos),
-					(size_t)fd_stat.st_size);
-			if (bytes == (ssize_t)fd_stat.st_size)
-				pos += fd_stat.st_size;
-			else {
-				printf("Error while reading %s\n",
-					fw_table[i].filename);
-				free(base);
-				exit(1);
-			}
-
-			close(fd);
-			pos = ALIGN(pos, 0x100U);
+			pos = ALIGN(pos + bytes, 0x100U);
    		count++;
    	} else {
    		/* This APU doesn't have this firmware. */