[coreboot-gerrit] Patch set updated for coreboot: cbfstool: Use buffer over offset/size pair for cbfs_copy_instance

[Aaron Durbin (adurbin@chromium.org)]

Aaron Durbin (adurbin at chromium.org) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/12787

-gerrit

commit ee91901ef7cf832bcb9cc4920f545c29a03708d4
Author: Patrick Georgi <pgeorgi at google.com>
Date:   Fri Nov 20 19:22:50 2015 +0100

    cbfstool: Use buffer over offset/size pair for cbfs_copy_instance
    
    This allows adding support for FMAP based cbfstool copy more easily.
    
    BUG=chromium:445938
    
    Change-Id: I72e7bc4da7d27853e324400f76f86136e3d8726e
    Signed-off-by: Patrick Georgi <pgeorgi at google.com>
---
 util/cbfstool/cbfs_image.c | 41 +++++++++++++----------------------------
 util/cbfstool/cbfs_image.h |  3 +--
 util/cbfstool/cbfstool.c   | 33 ++++++++++++++++++++-------------
 3 files changed, 34 insertions(+), 43 deletions(-)

diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index 1cfe90d..25aafe9 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -387,8 +387,7 @@ int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in,
 	return 1;
 }
 
-int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset,
-			size_t copy_size)
+int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst)
 {
 	assert(image);
 	if (!cbfs_is_legacy_cbfs(image))
@@ -399,37 +398,23 @@ int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset,
 	size_t align, entry_offset;
 	ssize_t last_entry_size;
 
-	size_t cbfs_offset, cbfs_end;
-	size_t copy_end = copy_offset + copy_size;
+	size_t copy_end = buffer_size(dst);
 
 	align = image->header.align;
 
-	cbfs_offset = image->header.offset;
-	cbfs_end = image->header.romsize;
-
-	if (copy_end > image->buffer.size) {
-		ERROR("Copy offset out of range: [%zx:%zx)\n",
-			copy_offset, copy_end);
-		return 1;
-	}
-
-	/* Range check requested copy region with source cbfs. */
-	if ((copy_offset >= cbfs_offset && copy_offset < cbfs_end) ||
-	    (copy_end >= cbfs_offset && copy_end <= cbfs_end)) {
-		ERROR("New image would overlap old one.\n");
-		return 1;
-	}
-
 	/* This will work, let's create a copy. */
-	copy_header = (struct cbfs_header *)(image->buffer.data + copy_offset);
+	copy_header = (struct cbfs_header *)buffer_get(dst);
 	cbfs_put_header(copy_header, &image->header);
 
 	copy_header->bootblocksize = 0;
-	/* Romsize is a misnomer. It's the absolute limit of cbfs content.*/
-	copy_header->romsize = htonl(copy_end);
-	entry_offset = align_up(copy_offset + sizeof(*copy_header), align);
-	copy_header->offset = htonl(entry_offset);
-	dst_entry = (struct cbfs_file *)(image->buffer.data + entry_offset);
+	/* FIXME: romsize and offset have a full-flash interpretation even
+	 * though they don't need to and should be region-relative (where
+	 * region is sufficiently specified by the master header pointer.
+	 * But that's for a later change. */
+	copy_header->romsize = htonl(dst->offset + buffer_size(dst));
+	entry_offset = align_up(sizeof(*copy_header), align);
+	copy_header->offset = htonl(dst->offset + entry_offset);
+	dst_entry = (struct cbfs_file *)(buffer_get(dst) + entry_offset);
 
 	/* Copy non-empty files */
 	for (src_entry = cbfs_find_first_entry(image);
@@ -446,7 +431,7 @@ int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset,
 		dst_entry = (struct cbfs_file *)(
 			(uintptr_t)dst_entry + align_up(entry_size, align));
 
-		if ((size_t)((char *)dst_entry - image->buffer.data) >=
+		if ((size_t)((void *)dst_entry - buffer_get(dst)) >=
 								copy_end) {
 			ERROR("Ran out of room in copy region.\n");
 			return 1;
@@ -454,7 +439,7 @@ int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset,
 	}
 
 	/* Last entry size is all the room above it. */
-	last_entry_size = copy_end - ((char *)dst_entry - image->buffer.data)
+	last_entry_size = copy_end - ((void *)dst_entry - buffer_get(dst))
 		- cbfs_calculate_file_header_size("");
 
 	if (last_entry_size < 0)
diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h
index 195494c..00adcc8 100644
--- a/util/cbfstool/cbfs_image.h
+++ b/util/cbfstool/cbfs_image.h
@@ -74,8 +74,7 @@ int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in,
 
 /* Create a duplicate CBFS image. Returns 0 on success, otherwise non-zero.
  * Will not succeed on new-style images without a master header. */
-int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset,
-			size_t copy_size);
+int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst);
 
 /* Releases the CBFS image. Returns 0 on success, otherwise non-zero. */
 int cbfs_image_delete(struct cbfs_image *image);
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 2529296..c9a9564 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -917,27 +917,34 @@ static int cbfs_update_fit(void)
 
 static int cbfs_copy(void)
 {
-	if (!param.copyoffset_assigned) {
-		ERROR("You need to specify -D/--copy-offset.\n");
-		return 1;
-	}
-
-	if (!param.size) {
-		ERROR("You need to specify -s/--size.\n");
-		return 1;
-	}
+	/* always a valid source region */
+	struct cbfs_image src_image;
 
-	struct cbfs_image image;
-	if (cbfs_image_from_buffer(&image, param.image_region,
+	if (cbfs_image_from_buffer(&src_image, param.image_region,
 							param.headeroffset))
 		return 1;
 
-	if (!cbfs_is_legacy_cbfs(&image)) {
+	struct buffer dst_buffer;
+
+	if (cbfs_is_legacy_cbfs(&src_image)) {
+		if (!param.copyoffset_assigned) {
+			ERROR("You need to specify -D/--copy-offset.\n");
+			return 1;
+		}
+
+		if (!param.size) {
+			ERROR("You need to specify -s/--size.\n");
+			return 1;
+		}
+
+		buffer_splice(&dst_buffer, param.image_region,
+			param.copyoffset, param.size);
+	} else {
 		ERROR("This operation is only valid on legacy images having CBFS master headers\n");
 		return 1;
 	}
 
-	return cbfs_copy_instance(&image, param.copyoffset, param.size);
+	return cbfs_copy_instance(&src_image, &dst_buffer);
 }
 
 static const struct command commands[] = {