Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/33515 )
Change subject: layout: Make `romentry.name` a pointer ......................................................................
layout: Make `romentry.name` a pointer
This should provide more flexibility while we don't have to allocate 256B extra per layout entry.
Change-Id: Ibb903113550ec13f43cbbd0a412c8f35fe1cf454 Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/flashrom/+/33515 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M flashrom.c M ich_descriptors.c M layout.c M layout.h M libflashrom.c 5 files changed, 39 insertions(+), 17 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/flashrom.c b/flashrom.c index d6c4d24..2d3cd18 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1338,7 +1338,11 @@ fallback->entry.start = 0; fallback->entry.end = flash->chip->total_size * 1024 - 1; fallback->entry.included = true; - strcpy(fallback->entry.name, "complete flash"); + fallback->entry.name = strdup("complete flash"); + if (!fallback->entry.name) { + msg_cerr("Failed to probe chip: %s\n", strerror(errno)); + return -1; + }
tmp = flashbuses_to_text(flash->chip->bustype); msg_cinfo("%s %s flash chip "%s" (%d kB, %s) ", force ? "Assuming" : "Found", diff --git a/ich_descriptors.c b/ich_descriptors.c index 251f636..0a4cf75 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -1153,7 +1153,8 @@ * @param len The length of the descriptor dump. * * @return 0 on success, - * 1 if the descriptor couldn't be parsed. + * 1 if the descriptor couldn't be parsed, + * 2 when out of memory. */ int layout_from_ich_descriptors(struct ich_layout *const layout, const void *const dump, const size_t len) { @@ -1178,7 +1179,9 @@ layout->entries[j].start = base; layout->entries[j].end = limit; layout->entries[j].included = false; - snprintf(layout->entries[j].name, sizeof(layout->entries[j].name), "%s", regions[i]); + layout->entries[j].name = strdup(regions[i]); + if (!layout->entries[j].name) + return 2; ++j; } layout->base.entries = layout->entries; diff --git a/layout.c b/layout.c index 6963e61..52aa388 100644 --- a/layout.c +++ b/layout.c @@ -15,6 +15,7 @@ * GNU General Public License for more details. */
+#include <errno.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -44,8 +45,8 @@ { struct flashrom_layout *const layout = get_global_layout(); FILE *romlayout; - char tempstr[256]; - int i; + char tempstr[256], tempname[256]; + int i, ret = 1;
romlayout = fopen(name, "r");
@@ -61,10 +62,9 @@ if (layout->num_entries >= MAX_ROMLAYOUT) { msg_gerr("Maximum number of ROM images (%i) in layout " "file reached.\n", MAX_ROMLAYOUT); - (void)fclose(romlayout); - return 1; + goto _close_ret; } - if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, layout->entries[layout->num_entries].name)) + if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, tempname)) continue; #if 0 // fscanf does not like arbitrary comments like that :( later @@ -76,12 +76,16 @@ tstr2 = strtok(NULL, ":"); if (!tstr1 || !tstr2) { msg_gerr("Error parsing layout file. Offending string: "%s"\n", tempstr); - (void)fclose(romlayout); - return 1; + goto _close_ret; } layout->entries[layout->num_entries].start = strtol(tstr1, (char **)NULL, 16); layout->entries[layout->num_entries].end = strtol(tstr2, (char **)NULL, 16); layout->entries[layout->num_entries].included = 0; + layout->entries[layout->num_entries].name = strdup(tempname); + if (!layout->entries[layout->num_entries].name) { + msg_gerr("Error adding layout entry: %s\n", strerror(errno)); + goto _close_ret; + } layout->num_entries++; }
@@ -91,9 +95,11 @@ layout->entries[i].end, layout->entries[i].name); }
- (void)fclose(romlayout); + ret = 0;
- return 0; +_close_ret: + (void)fclose(romlayout); + return ret; } #endif
@@ -197,6 +203,7 @@ }
for (i = 0; i < layout->num_entries; i++) { + free(layout->entries[i].name); layout->entries[i].included = 0; } layout->num_entries = 0; @@ -278,9 +285,13 @@ */ void flashrom_layout_release(struct flashrom_layout *const layout) { - if (layout == get_global_layout()) + unsigned int i; + + if (!layout || layout == get_global_layout()) return;
+ for (i = 0; i < layout->num_entries; ++i) + free(layout->entries[i].name); free(layout); }
diff --git a/layout.h b/layout.h index 0f07e09..b9454a8 100644 --- a/layout.h +++ b/layout.h @@ -39,7 +39,7 @@ chipoff_t start; chipoff_t end; bool included; - char name[256]; + char *name; };
struct flashrom_layout { diff --git a/libflashrom.c b/libflashrom.c index 721a11c..af62002 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -20,6 +20,7 @@ * Have a look at the Modules section for a function reference. */
+#include <errno.h> #include <stdlib.h> #include <string.h> #include <stdarg.h> @@ -384,9 +385,12 @@ l->entries[l->num_entries].start = fmap->areas[i].offset; l->entries[l->num_entries].end = fmap->areas[i].offset + fmap->areas[i].size - 1; l->entries[l->num_entries].included = false; - memset(l->entries[l->num_entries].name, 0, sizeof(l->entries[i].name)); - memcpy(l->entries[l->num_entries].name, fmap->areas[i].name, - min(FMAP_STRLEN, sizeof(l->entries[i].name))); + l->entries[l->num_entries].name = + strndup((const char *)fmap->areas[i].name, FMAP_STRLEN); + if (!l->entries[l->num_entries].name) { + msg_gerr("Error adding layout entry: %s\n", strerror(errno)); + return 1; + } msg_gdbg("fmap %08x - %08x named %s\n", l->entries[l->num_entries].start, l->entries[l->num_entries].end,