[coreboot-gerrit] Change in coreboot[master]: cbfstool: Use converted buffer size for do_cbfs_locate()

[Julius Werner (Code Review)]

Attention is currently required from: Raul Rangel, Patrick Georgi.
Hello Raul Rangel, Patrick Georgi,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/60084

to review the following change.

Change subject: cbfstool: Use converted buffer size for do_cbfs_locate()
......................................................................

cbfstool: Use converted buffer size for do_cbfs_locate()

The whole point of moving do_cbfs_locate() later (CB:59877) was that it
could use the file size that is actually going to be inserted into CBFS,
rather than the on-disk file size. Unfortunately, after all that work I
forgot to actually make it do that. This patch fixes that.

Since there is no more use case for do_cbfs_locate() having to figure
out the file size on its own, and that generally seems to be a bad idea
(as the original issue shows), also remove that part of it completely
and make the data_size parameter mandatory.

Signed-off-by: Julius Werner 
Change-Id: I1af35e8e388f78aae3593c029afcfb4e510d2b8f
---
M util/cbfstool/cbfstool.c
1 file changed, 6 insertions(+), 18 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/60084/1

diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 77460d1..a84601e 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -511,11 +511,6 @@
 {
    uint32_t metadata_size = 0;

-	if (!param.filename) {
-		ERROR("You need to specify -f/--filename.\n");
-		return 1;
-	}
-
    if (!param.name) {
    	ERROR("You need to specify -n/--name.\n");
    	return 1;
@@ -530,17 +525,10 @@
    	WARN("'%s' already in CBFS.\n", param.name);

if (!data_size) {
-		struct buffer buffer;
-		if (buffer_from_file(&buffer, param.filename) != 0) {
-			ERROR("Cannot load %s.\n", param.filename);
-			return 1;
-		}
-		data_size = buffer.size;
-		buffer_delete(&buffer);
+		ERROR("File '%s' is empty?\n", param.name);
+		return 1;
    }

-	DEBUG("File size is %zd (0x%zx)\n", data_size, data_size);
-
    /* Compute required page size */
    if (param.force_pow2_pagesize) {
    	param.pagesize = 1;
@@ -571,8 +559,8 @@
    					param.alignment, metadata_size);

if (address < 0) {
-		ERROR("'%s' can't fit in CBFS for page-size %#x, align %#x.\n",
-		      param.name, param.pagesize, param.alignment);
+		ERROR("'%s'(%u + %zu) can't fit in CBFS for page-size %#x, align %#x.\n",
+		      param.name, metadata_size, data_size, param.pagesize, param.alignment);
    	return 1;
    }

@@ -917,7 +905,7 @@

/* This needs to run after convert() to take compression into account. */
    if (!offset && param.alignment)
-		if (do_cbfs_locate(&offset, 0))
+		if (do_cbfs_locate(&offset, buffer_size(&buffer)))
    		goto error;

/* This needs to run after convert() to hash the actual final file data. */
@@ -1076,7 +1064,7 @@
    if (param.stage_xip) {
    	if (!param.baseaddress_assigned) {
    		param.alignment = 4*1024;
-			if (do_cbfs_locate(offset, 0))
+			if (do_cbfs_locate(offset, buffer_size(buffer)))
    			return -1;
    	}
    	assert(!IS_HOST_SPACE_ADDRESS(*offset));