Nico Huber merged this change.

View Change

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
layout.c: Don't use global variables for included regions

This removes the use of global variables for included region arguments
and also uses a linked list to store the arguments.

Change-Id: I6534cc58b8dcc6256c2730c809286d8083669a6c
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/31247
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
---
M cli_classic.c
M flash.h
M layout.c
M layout.h
4 files changed, 58 insertions(+), 51 deletions(-)

diff --git a/cli_classic.c b/cli_classic.c
index ced08c6..2e07612 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -151,6 +151,7 @@
#endif /* !STANDALONE */
char *tempstr = NULL;
char *pparam = NULL;
+ struct layout_include_args *include_args = NULL;

flashrom_set_log_callback((flashrom_log_callback *)&flashrom_print_cb);

@@ -290,7 +291,7 @@
break;
case 'i':
tempstr = strdup(optarg);
- if (register_include_arg(tempstr)) {
+ if (register_include_arg(&include_args, tempstr)) {
free(tempstr);
cli_classic_abort_usage();
}
@@ -452,7 +453,7 @@
goto out;
}

- if (!ifd && !fmap && process_include_args(get_global_layout())) {
+ if (!ifd && !fmap && process_include_args(get_global_layout(), include_args)) {
ret = 1;
goto out;
}
@@ -608,7 +609,7 @@
if (layoutfile) {
layout = get_global_layout();
} else if (ifd && (flashrom_layout_read_from_ifd(&layout, fill_flash, NULL, 0) ||
- process_include_args(layout))) {
+ process_include_args(layout, include_args))) {
ret = 1;
goto out_shutdown;
} else if (fmap && fmapfile) {
@@ -633,14 +634,14 @@
}

if (flashrom_layout_read_fmap_from_buffer(&layout, fill_flash, fmapfile_buffer, fmapfile_size) ||
- process_include_args(layout)) {
+ process_include_args(layout, include_args)) {
ret = 1;
free(fmapfile_buffer);
goto out_shutdown;
}
free(fmapfile_buffer);
} else if (fmap && (flashrom_layout_read_fmap_from_rom(&layout, fill_flash, 0,
- fill_flash->chip->total_size * 1024) || process_include_args(layout))) {
+ fill_flash->chip->total_size * 1024) || process_include_args(layout, include_args))) {
ret = 1;
goto out_shutdown;
}
@@ -675,7 +676,7 @@
for (i = 0; i < chipcount; i++)
free(flashes[i].chip);

- layout_cleanup();
+ layout_cleanup(&include_args);
free(filename);
free(fmapfile);
free(referencefile);
diff --git a/flash.h b/flash.h
index 911214c..5bbfa0a 100644
--- a/flash.h
+++ b/flash.h
@@ -391,10 +391,10 @@
#define msg_cspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* chip debug spew */

/* layout.c */
-int register_include_arg(char *name);
+int register_include_arg(struct layout_include_args **args, char *name);
int read_romlayout(const char *name);
int normalize_romentries(const struct flashctx *flash);
-void layout_cleanup(void);
+void layout_cleanup(struct layout_include_args **args);

/* spi.c */
struct spi_command {
diff --git a/layout.c b/layout.c
index f757783..52a0c2e 100644
--- a/layout.c
+++ b/layout.c
@@ -26,11 +26,6 @@
struct romentry entries[MAX_ROMLAYOUT];
static struct flashrom_layout layout = { entries, 0 };

-/* include_args holds the arguments specified at the command line with -i. They must be processed at some point
- * so that desired regions are marked as "included" in the layout. */
-static char *include_args[MAX_ROMLAYOUT];
-static int num_include_args = 0; /* the number of valid include_args. */
-
struct flashrom_layout *get_global_layout(void)
{
return &layout;
@@ -101,37 +96,34 @@
}
#endif

-/* returns the index of the entry (or a negative value if it is not found) */
-static int find_include_arg(const char *const name)
-{
- unsigned int i;
- for (i = 0; i < num_include_args; i++) {
- if (!strcmp(include_args[i], name))
- return i;
- }
- return -1;
-}
-
/* register an include argument (-i) for later processing */
-int register_include_arg(char *name)
+int register_include_arg(struct layout_include_args **args, char *name)
{
- if (num_include_args >= MAX_ROMLAYOUT) {
- msg_gerr("Too many regions included (%i).\n", num_include_args);
- return 1;
- }
-
+ struct layout_include_args *tmp;
if (name == NULL) {
msg_gerr("<NULL> is a bad region name.\n");
return 1;
}

- if (find_include_arg(name) != -1) {
- msg_gerr("Duplicate region name: \"%s\".\n", name);
+ tmp = *args;
+ while (tmp) {
+ if (!strcmp(tmp->name, name)) {
+ msg_gerr("Duplicate region name: \"%s\".\n", name);
+ return 1;
+ }
+ tmp = tmp->next;
+ }
+
+ tmp = malloc(sizeof(struct layout_include_args));
+ if (tmp == NULL) {
+ msg_gerr("Could not allocate memory");
return 1;
}

- include_args[num_include_args] = name;
- num_include_args++;
+ tmp->name = name;
+ tmp->next = *args;
+ *args = tmp;
+
return 0;
}

@@ -153,45 +145,54 @@
/* process -i arguments
* returns 0 to indicate success, >0 to indicate failure
*/
-int process_include_args(struct flashrom_layout *const l)
+int process_include_args(struct flashrom_layout *l, const struct layout_include_args *const args)
{
- int i;
+ int found = 0;
+ const struct layout_include_args *tmp;

- if (num_include_args == 0)
+ if (args == NULL)
return 0;

/* User has specified an area, but no layout file is loaded. */
if (l->num_entries == 0) {
msg_gerr("Region requested (with -i \"%s\"), "
"but no layout data is available.\n",
- include_args[0]);
+ args->name);
return 1;
}

- for (i = 0; i < num_include_args; i++) {
- if (find_romentry(l, include_args[i]) < 0) {
+ tmp = args;
+ while (tmp) {
+ if (find_romentry(l, tmp->name) < 0) {
msg_gerr("Invalid region specified: \"%s\".\n",
- include_args[i]);
+ tmp->name);
return 1;
}
+ tmp = tmp->next;
+ found++;
}

- msg_ginfo("Using region%s: \"%s\"", num_include_args > 1 ? "s" : "",
- include_args[0]);
- for (i = 1; i < num_include_args; i++)
- msg_ginfo(", \"%s\"", include_args[i]);
+ msg_ginfo("Using region%s:", found > 1 ? "s" : "");
+ tmp = args;
+ while (tmp) {
+ msg_ginfo(" \"%s\"%s", tmp->name, found > 1 ? "," : "");
+ found--;
+ tmp = tmp->next;
+ }
msg_ginfo(".\n");
return 0;
}

-void layout_cleanup(void)
+void layout_cleanup(struct layout_include_args **args)
{
int i;
- for (i = 0; i < num_include_args; i++) {
- free(include_args[i]);
- include_args[i] = NULL;
+ struct layout_include_args *tmp;
+
+ while (*args) {
+ tmp = (*args)->next;
+ free(*args);
+ *args = tmp;
}
- num_include_args = 0;

for (i = 0; i < layout.num_entries; i++) {
layout.entries[i].included = 0;
diff --git a/layout.h b/layout.h
index 5c07407..0f07e09 100644
--- a/layout.h
+++ b/layout.h
@@ -54,11 +54,16 @@
struct romentry entry;
};

+struct layout_include_args {
+ char *name;
+ struct layout_include_args *next;
+};
+
struct flashrom_layout *get_global_layout(void);
struct flashrom_flashctx;
const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx);

-int process_include_args(struct flashrom_layout *);
+int process_include_args(struct flashrom_layout *l, const struct layout_include_args *const args);
const struct romentry *layout_next_included_region(const struct flashrom_layout *, chipoff_t);

#endif /* !__LAYOUT_H__ */

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

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6534cc58b8dcc6256c2730c809286d8083669a6c
Gerrit-Change-Number: 31247
Gerrit-PatchSet: 7
Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Thomas Heijligen <src@posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged