[coreboot-gerrit] Patch set updated for coreboot: 1ceb56a cbfstool: Deserialize CBFS master header when reading image

Alexandru Gagniuc (mr.nuke.me@gmail.com) gerrit at coreboot.org
Thu Feb 6 02:46:28 CET 2014


Alexandru Gagniuc (mr.nuke.me at gmail.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/5121

-gerrit

commit 1ceb56af55fcc55187bffd62d2bd33b4f44601be
Author: Alexandru Gagniuc <mr.nuke.me at gmail.com>
Date:   Wed Feb 5 01:10:08 2014 -0600

    cbfstool: Deserialize CBFS master header when reading image
    
    Rather than  using [hn]to[nh] whenever accessing a member of the CBFS
    header, deserialize the header when opening the CBFS image. The header
    is no longer a pointer inside the CBFS buffer, but a separate struct,
    a copy of the original header in a host-friendly format. This kills
    more of the ntohl usage.
    
    Change-Id: I5f8a5818b9d5a2d1152b1906249c4a5847d02bac
    Signed-off-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
---
 util/cbfstool/cbfs_image.c | 85 ++++++++++++++++++++++++++++++----------------
 util/cbfstool/cbfs_image.h |  2 ++
 util/cbfstool/cbfstool.c   |  2 +-
 3 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index dddd480..6ecffd1 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -113,7 +113,7 @@ static int cbfs_calculate_file_header_size(const char *name)
 		align_up(strlen(name) + 1, CBFS_FILENAME_ALIGN));
 }
 
-static int cbfs_fix_legacy_size(struct cbfs_image *image)
+static int cbfs_fix_legacy_size(struct cbfs_image *image, char *hdr_loc)
 {
 	// A bug in old cbfstool may produce extra few bytes (by alignment) and
 	// cause cbfstool to overwrite things after free space -- which is
@@ -125,12 +125,11 @@ static int cbfs_fix_legacy_size(struct cbfs_image *image)
 	     entry = cbfs_find_next_entry(image, entry)) {
 		last = entry;
 	}
-	if ((char *)first < (char *)image->header &&
-	    (char *)entry > (char *)image->header) {
+	if ((char *)first < (char *)hdr_loc &&
+	    (char *)entry > (char *)hdr_loc) {
 		WARN("CBFS image was created with old cbfstool with size bug. "
 		     "Fixing size in last entry...\n");
-		last->len = htonl(ntohl(last->len) -
-				  ntohl(image->header->align));
+		last->len = htonl(ntohl(last->len) - image->header->align);
 		DEBUG("Last entry has been changed from 0x%x to 0x%x.\n",
 		      cbfs_get_entry_addr(image, entry),
 		      cbfs_get_entry_addr(image,
@@ -154,6 +153,23 @@ void cbfs_put_header(void *dest, const struct cbfs_header *header)
 	xdr_be.put32(&outheader, header->offset);
 	xdr_be.put32(&outheader, header->architecture);
 }
+
+void cbfs_get_header(struct cbfs_header *header, const void *src)
+{
+	struct buffer outheader;
+
+	outheader.data = (void *)src;	/* We're not modifying the data */
+	outheader.size = 0;
+
+	header->magic = xdr_be.get32(&outheader);
+	header->version = xdr_be.get32(&outheader);
+	header->romsize = xdr_be.get32(&outheader);
+	header->bootblocksize = xdr_be.get32(&outheader);
+	header->align = xdr_be.get32(&outheader);
+	header->offset = xdr_be.get32(&outheader);
+	header->architecture = xdr_be.get32(&outheader);
+}
+
 int cbfs_image_create(struct cbfs_image *image,
 		      uint32_t myarch,
 		      size_t size,
@@ -167,6 +183,7 @@ int cbfs_image_create(struct cbfs_image *image,
 	struct cbfs_file *entry;
 	uint32_t cbfs_len;
 	size_t entry_header_len;
+	void *header_loc;
 
 	DEBUG("cbfs_image_create: bootblock=0x%x+0x%zx, "
 	      "header=0x%x+0x%zx, entries_offset=0x%x\n",
@@ -175,7 +192,8 @@ int cbfs_image_create(struct cbfs_image *image,
 
 	if (buffer_create(&image->buffer, size, "(new)") != 0)
 		return -1;
-	image->header = NULL;
+	if ((image->header = malloc(sizeof(*image->header))) == NULL)
+		return -1;
 	memset(image->buffer.data, CBFS_CONTENT_DEFAULT_VALUE, size);
 
 	// Adjust legcay top-aligned address to ROM offset.
@@ -214,15 +232,16 @@ int cbfs_image_create(struct cbfs_image *image,
 		      header_offset, sizeof(header), size);
 		return -1;
 	}
-	image->header = (struct cbfs_header *)(image->buffer.data + header_offset);
-	header.magic = CBFS_HEADER_MAGIC;
-	header.version = CBFS_HEADER_VERSION;
-	header.romsize = size;
-	header.bootblocksize = bootblock->size;
-	header.align = align;
-	header.offset = entries_offset;
-	header.architecture = myarch;
-	cbfs_put_header(image->header, &header);
+	image->header->magic = CBFS_HEADER_MAGIC;
+	image->header->version = CBFS_HEADER_VERSION;
+	image->header->romsize = size;
+	image->header->bootblocksize = bootblock->size;
+	image->header->align = align;
+	image->header->offset = entries_offset;
+	image->header->architecture = myarch;
+
+	header_loc = (image->buffer.data + header_offset);
+	cbfs_put_header(header_loc, image->header);
 
 	// Prepare entries
 	if (align_up(entries_offset, align) != entries_offset) {
@@ -252,18 +271,24 @@ int cbfs_image_create(struct cbfs_image *image,
 
 int cbfs_image_from_file(struct cbfs_image *image, const char *filename)
 {
+	void *header_loc;
+
 	if (buffer_from_file(&image->buffer, filename) != 0)
 		return -1;
 	DEBUG("read_cbfs_image: %s (%zd bytes)\n", image->buffer.name,
 	      image->buffer.size);
-	image->header = cbfs_find_header(image->buffer.data,
-					 image->buffer.size);
-	if (!image->header) {
+	header_loc = cbfs_find_header(image->buffer.data, image->buffer.size);
+	if (!header_loc) {
 		ERROR("%s does not have CBFS master header.\n", filename);
 		cbfs_image_delete(image);
 		return -1;
 	}
-	cbfs_fix_legacy_size(image);
+
+	if ((image->header = malloc(sizeof(*image->header))) == NULL)
+		return -1;
+
+	cbfs_get_header(image->header, header_loc);
+	cbfs_fix_legacy_size(image, header_loc);
 
 	return 0;
 }
@@ -296,7 +321,7 @@ static int cbfs_add_entry_at(struct cbfs_image *image,
 	uint32_t header_size = cbfs_calculate_file_header_size(name),
 		 min_entry_size = cbfs_calculate_file_header_size("");
 	uint32_t len, target;
-	uint32_t align = ntohl(image->header->align);
+	uint32_t align = image->header->align;
 
 	target = content_offset - header_size;
 	if (target % align)
@@ -372,7 +397,7 @@ int cbfs_add_entry(struct cbfs_image *image, struct buffer *buffer,
 
 	if (IS_TOP_ALIGNED_ADDRESS(content_offset)) {
 		// legacy cbfstool takes top-aligned address.
-		uint32_t theromsize = ntohl(image->header->romsize);
+		uint32_t theromsize = image->header->romsize;
 		INFO("Converting top-aligned address 0x%x to offset: 0x%x\n",
 		     content_offset, content_offset + theromsize);
 		content_offset += theromsize;
@@ -546,10 +571,10 @@ int cbfs_print_header_info(struct cbfs_image *image)
 	       "alignment: %d bytes\n\n",
 	       basename(name),
 	       image->buffer.size / 1024,
-	       ntohl(image->header->bootblocksize),
-	       ntohl(image->header->romsize),
-	       ntohl(image->header->offset),
-	       ntohl(image->header->align));
+	       image->header->bootblocksize,
+	       image->header->romsize,
+	       image->header->offset,
+	       image->header->align);
 	free(name);
 	return 0;
 }
@@ -779,14 +804,14 @@ struct cbfs_file *cbfs_find_first_entry(struct cbfs_image *image)
 {
 	assert(image && image->header);
 	return (struct cbfs_file *)(image->buffer.data +
-				   ntohl(image->header->offset));
+				   image->header->offset);
 }
 
 struct cbfs_file *cbfs_find_next_entry(struct cbfs_image *image,
 				       struct cbfs_file *entry)
 {
 	uint32_t addr = cbfs_get_entry_addr(image, entry);
-	int align = ntohl(image->header->align);
+	int align = image->header->align;
 	assert(entry && cbfs_is_valid_entry(image, entry));
 	addr += ntohl(entry->offset) + ntohl(entry->len);
 	addr = align_up(addr, align);
@@ -850,7 +875,7 @@ int32_t cbfs_locate_entry(struct cbfs_image *image, const char *name,
 
 	/* Default values: allow fitting anywhere in ROM. */
 	if (!page_size)
-		page_size = ntohl(image->header->romsize);
+		page_size = image->header->romsize;
 	if (!align)
 		align = 1;
 
@@ -858,9 +883,9 @@ int32_t cbfs_locate_entry(struct cbfs_image *image, const char *name,
 		ERROR("Input file size (%d) greater than page size (%d).\n",
 		      size, page_size);
 
-	if (page_size % ntohl(image->header->align))
+	if (page_size % image->header->align)
 		WARN("%s: Page size (%#x) not aligned with CBFS image (%#x).\n",
-		     __func__, page_size, ntohl(image->header->align));
+		     __func__, page_size, image->header->align);
 
 	/* TODO Old cbfstool always assume input is a stage file (and adding
 	 * sizeof(cbfs_stage) for header. We should fix that by adding "-t"
diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h
index 8ea4e47..0a05eb2 100644
--- a/util/cbfstool/cbfs_image.h
+++ b/util/cbfstool/cbfs_image.h
@@ -34,6 +34,8 @@ struct cbfs_image {
 /* Given a pointer, serialize the header from host-native byte format
  * to cbfs format, i.e. big-endian. */
 void cbfs_put_header(void *dest, const struct cbfs_header *header);
+/* Or deserialize into host-native format */
+void cbfs_get_header(struct cbfs_header *header, const void *src);
 
 /* Creates an empty CBFS image by given size, and description to its content
  * (bootblock, align, header location, starting offset of CBFS entries.
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 1d93981..792dd91 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -436,7 +436,7 @@ static int cbfs_locate(void)
 	}
 
 	if (param.top_aligned)
-		address = address - ntohl(image.header->romsize);
+		address = address - image.header->romsize;
 
 	cbfs_image_delete(&image);
 	printf("0x%x\n", address);



More information about the coreboot-gerrit mailing list