Peter Marheine has uploaded this change for review.

View Change

libflashrom: clear layout on flashrom_layout_release

This changes the behavior of flashrom_layout_release to always clear the
provided layout, even if it is the global layout (as is currently always
the case).

Not clearing it causes confusing behavior where a user may release a
layout and create a new one, but because it's always the global layout
which wasn't cleared there will be stale information in the returned
layout. This can easily lead to writing unexpected flash regions because
the API provides no way to stop including regions, and makes it
effectively impossible to stop including a region without completely
tearing down the library.

Change-Id: I8ba4b356f2a40ccbfd4e91f81af01d5f6a6de515
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
---
M cli_classic.c
M flash.h
M layout.c
3 files changed, 7 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/54147/1
diff --git a/cli_classic.c b/cli_classic.c
index 5fd8092..9dcfe71 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -852,7 +852,7 @@
for (i = 0; i < chipcount; i++)
free(flashes[i].chip);

- layout_cleanup(&include_args);
+ layout_cleanup_args(&include_args);
free(filename);
free(fmapfile);
free(referencefile);
diff --git a/flash.h b/flash.h
index dd8e156..fbc3539 100644
--- a/flash.h
+++ b/flash.h
@@ -421,7 +421,7 @@
int register_include_arg(struct layout_include_args **args, const char *arg);
int read_romlayout(const char *name);
int normalize_romentries(const struct flashctx *flash);
-void layout_cleanup(struct layout_include_args **args);
+void layout_cleanup_args(struct layout_include_args **args);

/* spi.c */
struct spi_command {
diff --git a/layout.c b/layout.c
index 4d1dd56..f81e01d 100644
--- a/layout.c
+++ b/layout.c
@@ -276,10 +276,8 @@
return overlap_detected;
}

-void layout_cleanup(struct layout_include_args **args)
+void layout_cleanup_args(struct layout_include_args **args)
{
- struct flashrom_layout *const layout = get_global_layout();
- unsigned int i;
struct layout_include_args *tmp;

while (*args) {
@@ -289,13 +287,6 @@
free(*args);
*args = tmp;
}
-
- for (i = 0; i < layout->num_entries; i++) {
- free(layout->entries[i].name);
- free(layout->entries[i].file);
- layout->entries[i].included = false;
- }
- layout->num_entries = 0;
}

/* Validate and - if needed - normalize layout entries. */
@@ -405,14 +396,16 @@
{
unsigned int i;

- if (!layout || layout == get_global_layout())
+ if (!layout)
return;

for (i = 0; i < layout->num_entries; ++i) {
free(layout->entries[i].name);
free(layout->entries[i].file);
}
- free(layout);
+ layout->num_entries = 0;
+ if (layout != get_global_layout())
+ free(layout);
}

/** @} */ /* end flashrom-layout */

To view, visit change 54147. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8ba4b356f2a40ccbfd4e91f81af01d5f6a6de515
Gerrit-Change-Number: 54147
Gerrit-PatchSet: 1
Gerrit-Owner: Peter Marheine <pmarheine@chromium.org>
Gerrit-MessageType: newchange