[coreboot-gerrit] Patch set updated for coreboot: 9407f41 cbfstool: clean up source code

Patrick Georgi (pgeorgi@google.com) gerrit at coreboot.org
Sat Apr 18 08:47:36 CEST 2015


Patrick Georgi (pgeorgi at google.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/9746

-gerrit

commit 9407f4199b8d0d8e03b7644259a2680d7ca6721a
Author: Vadim Bendebury <vbendeb at chromium.org>
Date:   Tue Dec 23 15:59:57 2014 -0800

    cbfstool: clean up source code
    
    The following changes were made:
    
    - order commands and options definitions alphabetically
    - do not report errors at cbfs_image_from_file() call sites - the
      error is reported by the function itself
    - remove the unused parameter in cbfs_create_empty_entry() prototype
    
    BRANCH=storm
    BUG=none
    TEST=compiled cbfstool, built a storm image, observed that the image
         still boots
    
    Change-Id: I31b15fab0a63749c6f2d351901ed545de531eb39
    Signed-off-by: Patrick Georgi <pgeorgi at chromium.org>
    Original-Commit-Id: a909a50e03be77f972b1a497198fe758661aa9f8
    Original-Change-Id: I4b8898dbd44eeb2c6b388a485366e4e22b1bed16
    Original-Signed-off-by: Vadim Bendebury <vbendeb at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/237560
    Original-Reviewed-by: David Hendricks <dhendrix at chromium.org>
---
 util/cbfstool/cbfs_image.c | 21 ++++++-------
 util/cbfstool/cbfs_image.h |  2 +-
 util/cbfstool/cbfstool.c   | 78 +++++++++++++++++-----------------------------
 3 files changed, 40 insertions(+), 61 deletions(-)

diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index 9df2030..e3f3e4e 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -262,7 +262,7 @@ int cbfs_image_create(struct cbfs_image *image,
 	if (header_offset > entries_offset && header_offset < cbfs_len)
 		cbfs_len = header_offset;
 	cbfs_len -= entries_offset + align + entry_header_len;
-	cbfs_create_empty_entry(image, entry, cbfs_len, "");
+	cbfs_create_empty_entry(entry, cbfs_len, "");
 	LOG("Created CBFS image (capacity = %d bytes)\n", cbfs_len);
 	return 0;
 }
@@ -371,7 +371,7 @@ int cbfs_copy_instance(struct cbfs_image *image, size_t copy_offset,
 	if (last_entry_size < 0)
 		WARN("No room to create the last entry!\n")
 	else
-		cbfs_create_empty_entry(image, dst_entry, last_entry_size, "");
+		cbfs_create_empty_entry(dst_entry, last_entry_size, "");
 
 	return 0;
 }
@@ -421,14 +421,14 @@ static int cbfs_add_entry_at(struct cbfs_image *image,
 	if (target - addr > min_entry_size) {
 		DEBUG("|min|...|header|content|... <create new entry>\n");
 		len = target - addr - min_entry_size;
-		cbfs_create_empty_entry(image, entry, len, "");
+		cbfs_create_empty_entry(entry, len, "");
 		if (verbose > 1) cbfs_print_entry_info(image, entry, stderr);
 		entry = cbfs_find_next_entry(image, entry);
 		addr = cbfs_get_entry_addr(image, entry);
 	}
 
 	len = size + (content_offset - addr - header_size);
-	cbfs_create_empty_entry(image, entry, len, name);
+	cbfs_create_empty_entry(entry, len, name);
 	if (len != size) {
 		DEBUG("|..|header|content|... <use offset to create entry>\n");
 		DEBUG("before: offset=0x%x, len=0x%x\n",
@@ -462,7 +462,7 @@ static int cbfs_add_entry_at(struct cbfs_image *image,
 	}
 
 	len = addr_next - addr - min_entry_size;
-	cbfs_create_empty_entry(image, entry, len, "");
+	cbfs_create_empty_entry(entry, len, "");
 	if (verbose > 1) cbfs_print_entry_info(image, entry, stderr);
 	return 0;
 }
@@ -518,8 +518,7 @@ int cbfs_add_entry(struct cbfs_image *image, struct buffer *buffer,
 		if (!content_offset || content_offset == addr + header_size) {
 			DEBUG("Filling new entry data (%zd bytes).\n",
 			      buffer->size);
-			cbfs_create_empty_entry(image, entry, buffer->size,
-						name);
+			cbfs_create_empty_entry(entry, buffer->size, name);
 			entry->type = htonl(type);
 			memcpy(CBFS_SUBHEADER(entry), buffer->data, buffer->size);
 			if (verbose)
@@ -542,7 +541,7 @@ int cbfs_add_entry(struct cbfs_image *image, struct buffer *buffer,
 			}
 			new_size -= cbfs_calculate_file_header_size("");
 			DEBUG("new size: %d\n", new_size);
-			cbfs_create_empty_entry(image, entry, new_size, "");
+			cbfs_create_empty_entry(entry, new_size, "");
 			if (verbose)
 				cbfs_print_entry_info(image, entry, stderr);
 			return 0;
@@ -827,7 +826,7 @@ int cbfs_merge_empty_entry(struct cbfs_image *image, struct cbfs_file *entry,
 		DEBUG("join_empty_entry: combine 0x%x+0x%x and 0x%x+0x%x.\n",
 		      cbfs_get_entry_addr(image, entry), ntohl(entry->len),
 		      cbfs_get_entry_addr(image, next), ntohl(next->len));
-		cbfs_create_empty_entry(image, entry,
+		cbfs_create_empty_entry(entry,
 					(last_addr - addr -
 					 cbfs_calculate_file_header_size("")),
 					"");
@@ -943,8 +942,8 @@ int cbfs_is_valid_entry(struct cbfs_image *image, struct cbfs_file *entry)
 		       sizeof(entry->magic)) == 0);
 }
 
-int cbfs_create_empty_entry(struct cbfs_image *image, struct cbfs_file *entry,
-		      size_t len, const char *name)
+int cbfs_create_empty_entry(struct cbfs_file *entry,
+			    size_t len, const char *name)
 {
 	memset(entry, CBFS_CONTENT_DEFAULT_VALUE, sizeof(*entry));
 	memcpy(entry->magic, CBFS_FILE_MAGIC, sizeof(entry->magic));
diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h
index 1e5a99f..55ce028 100644
--- a/util/cbfstool/cbfs_image.h
+++ b/util/cbfstool/cbfs_image.h
@@ -84,7 +84,7 @@ int cbfs_remove_entry(struct cbfs_image *image, const char *name);
 
 /* Initializes a new empty (type = NULL) entry with size and name in CBFS image.
  * Returns 0 on success, otherwise (ex, not found) non-zero. */
-int cbfs_create_empty_entry(struct cbfs_image *image, struct cbfs_file *entry,
+int cbfs_create_empty_entry(struct cbfs_file *entry,
 			    size_t len, const char *name);
 
 /* Finds a location to put given content by specified criteria:
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index b60f199..3554cfa 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -144,10 +144,8 @@ static int cbfs_add_component(const char *cbfs_name,
 		return 1;
 	}
 
-	if (cbfs_image_from_file(&image, cbfs_name, headeroffset) != 0) {
-		ERROR("Could not load ROM image '%s'.\n", cbfs_name);
+	if (cbfs_image_from_file(&image, cbfs_name, headeroffset))
 		return 1;
-	}
 
 	if (buffer_from_file(&buffer, filename) != 0) {
 		ERROR("Could not load file '%s'.\n", filename);
@@ -319,12 +317,8 @@ static int cbfs_remove(void)
 		return 1;
 	}
 
-	if (cbfs_image_from_file(&image, param.cbfs_name,
-		param.headeroffset) != 0) {
-		ERROR("Could not load ROM image '%s'.\n",
-			param.cbfs_name);
+	if (cbfs_image_from_file(&image, param.cbfs_name, param.headeroffset))
 		return 1;
-	}
 
 	if (cbfs_remove_entry(&image, param.name) != 0) {
 		ERROR("Removing file '%s' failed.\n",
@@ -444,11 +438,8 @@ static int cbfs_locate(void)
 		return 1;
 	}
 
-	if (cbfs_image_from_file(&image, param.cbfs_name,
-		param.headeroffset) != 0) {
-		ERROR("Failed to load %s.\n", param.cbfs_name);
+	if (cbfs_image_from_file(&image, param.cbfs_name, param.headeroffset))
 		return 1;
-	}
 
 	if (cbfs_get_entry(&image, param.name))
 		WARN("'%s' already in CBFS.\n", param.name);
@@ -481,12 +472,9 @@ static int cbfs_locate(void)
 static int cbfs_print(void)
 {
 	struct cbfs_image image;
-	if (cbfs_image_from_file(&image, param.cbfs_name,
-		param.headeroffset) != 0) {
-		ERROR("Could not load ROM image '%s'.\n",
-			param.cbfs_name);
+	if (cbfs_image_from_file(&image, param.cbfs_name, param.headeroffset))
 		return 1;
-	}
+
 	cbfs_print_directory(&image);
 	cbfs_image_delete(&image);
 	return 0;
@@ -507,15 +495,11 @@ static int cbfs_extract(void)
 		return 1;
 	}
 
-	if (cbfs_image_from_file(&image, param.cbfs_name,
-		param.headeroffset) != 0) {
-		ERROR("Could not load ROM image '%s'.\n",
-			param.cbfs_name);
+	if (cbfs_image_from_file(&image, param.cbfs_name, param.headeroffset))
 		result = 1;
-	} else if (cbfs_export_entry(&image, param.name,
-				     param.filename) != 0) {
+	else if (cbfs_export_entry(&image, param.name,
+				   param.filename))
 		result = 1;
-	}
 
 	cbfs_image_delete(&image);
 	return result;
@@ -537,12 +521,8 @@ static int cbfs_update_fit(void)
 		return 1;
 	}
 
-	if (cbfs_image_from_file(&image, param.cbfs_name,
-		param.headeroffset) != 0) {
-		ERROR("Could not load ROM image '%s'.\n",
-			param.cbfs_name);
+	if (cbfs_image_from_file(&image, param.cbfs_name, param.headeroffset))
 		return 1;
-	}
 
 	ret = fit_update_table(&image, param.fit_empty_entries, param.name);
 	if (!ret)
@@ -580,43 +560,43 @@ static int cbfs_copy(void)
 
 static const struct command commands[] = {
 	{"add", "H;f:n:t:b:vh?", cbfs_add},
+	{"add-flat-binary", "H:f:n:l:e:c:b:vh?", cbfs_add_flat_binary},
 	{"add-payload", "H:f:n:t:c:b:vh?C:I:", cbfs_add_payload},
 	{"add-stage", "H:f:n:t:c:b:S:vh?", cbfs_add_stage},
-	{"add-flat-binary", "H:f:n:l:e:c:b:vh?", cbfs_add_flat_binary},
 	{"add-int", "H:i:n:b:vh?", cbfs_add_integer},
-	{"remove", "H:n:vh?", cbfs_remove},
-	{"copy", "H:D:s:", cbfs_copy},
 	{"create", "s:B:b:H:a:o:m:vh?", cbfs_create},
+	{"copy", "H:D:s:", cbfs_copy},
+	{"extract", "H:n:f:vh?", cbfs_extract},
 	{"locate", "H:f:n:P:a:Tvh?", cbfs_locate},
 	{"print", "H:vh?", cbfs_print},
-	{"extract", "H:n:f:vh?", cbfs_extract},
+	{"remove", "H:n:vh?", cbfs_remove},
 	{"update-fit", "H:n:x:vh?", cbfs_update_fit},
 };
 
 static struct option long_options[] = {
-	{"name",          required_argument, 0, 'n' },
-	{"type",          required_argument, 0, 't' },
-	{"compression",   required_argument, 0, 'c' },
+	{"alignment",     required_argument, 0, 'a' },
 	{"base-address",  required_argument, 0, 'b' },
-	{"load-address",  required_argument, 0, 'l' },
-	{"top-aligned",   required_argument, 0, 'T' },
+	{"bootblock",     required_argument, 0, 'B' },
+	{"cmdline",       required_argument, 0, 'C' },
+	{"compression",   required_argument, 0, 'c' },
 	{"copy-offset",   required_argument, 0, 'D' },
+	{"empty-fits",    required_argument, 0, 'x' },
 	{"entry-point",   required_argument, 0, 'e' },
-	{"size",          required_argument, 0, 's' },
-	{"bootblock",     required_argument, 0, 'B' },
-	{"header-offset", required_argument, 0, 'H' },
-	{"alignment",     required_argument, 0, 'a' },
-	{"page-size",     required_argument, 0, 'P' },
-	{"offset",        required_argument, 0, 'o' },
 	{"file",          required_argument, 0, 'f' },
+	{"header-offset", required_argument, 0, 'H' },
+	{"help",          no_argument,       0, 'h' },
+	{"ignore-sec",    required_argument, 0, 'S' },
+	{"initrd",        required_argument, 0, 'I' },
 	{"int",           required_argument, 0, 'i' },
+	{"load-address",  required_argument, 0, 'l' },
 	{"machine",       required_argument, 0, 'm' },
-	{"empty-fits",    required_argument, 0, 'x' },
-	{"initrd",        required_argument, 0, 'I' },
-	{"cmdline",       required_argument, 0, 'C' },
-	{"ignore-sec",    required_argument, 0, 'S' },
+	{"name",          required_argument, 0, 'n' },
+	{"offset",        required_argument, 0, 'o' },
+	{"page-size",     required_argument, 0, 'P' },
+	{"size",          required_argument, 0, 's' },
+	{"top-aligned",   required_argument, 0, 'T' },
+	{"type",          required_argument, 0, 't' },
 	{"verbose",       no_argument,       0, 'v' },
-	{"help",          no_argument,       0, 'h' },
 	{NULL,            0,                 0,  0  }
 };
 



More information about the coreboot-gerrit mailing list