util/amdfwtool: Consolidate entry line regex pattern
There are 2 regex patterns defined to process the lines from *fw.cfg:
1) for lines with mandatory entries
2) for lines with mandatory + optional entries
Consolidate the regex pattern. Add enums for matching regex caller
groups so that the human readable group IDs can be used instead of magic
numbers.
BUG=None
TEST=Build Skyrim BIOS which only have mandatory entries. Build Guybrush
BIOS image which have both mandatory and optional entries. Confirm that
the amdfw.rom built before and after this change have matching SHA in
both Skyrim and Guybrush images. This ensures that the optional level
entries in Guybrush are handled as expected. Boot to OS in Skyrim.
Change-Id: I7289ddbbec4d5daefe64f59b687ba3a4af46d052
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/74950
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Van Patten <timvp@google.com>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
diff --git a/util/amdfwtool/data_parse.c b/util/amdfwtool/data_parse.c
index 50f844f..b94e46f 100644
--- a/util/amdfwtool/data_parse.c
+++ b/util/amdfwtool/data_parse.c
@@ -29,22 +29,7 @@
"[[:space:]]+"
/* followed by a chunk of nonwhitespace for filename field */
"([^[:space:]]+)"
- /* followed by optional whitespace */
- "[[:space:]]*$";
-static regex_t entries_line_expr;
-
-static const char entries_lvl_line_regex[] =
- /* optional whitespace */
- "^[[:space:]]*"
- /* followed by a chunk of nonwhitespace for macro field */
- "([^[:space:]]+)"
- /* followed by one or more whitespace characters */
- "[[:space:]]+"
- /* followed by a chunk of nonwhitespace for filename field */
- "([^[:space:]]+)"
- /* followed by one or more whitespace characters */
- "[[:space:]]+"
- /* followed by a chunk of nonwhitespace for level field
+ /* followed by an optional whitespace + chunk of nonwhitespace for level field
1st char L: Indicator of field "level"
2nd char:
Directory level to be dropped in.
@@ -60,10 +45,18 @@
L12: Level 1 for normal mode, level 2 for A/B mode
Lx1: Use default value for normal mode, level 1 for A/B mode
*/
- "([Ll][12bxBX]{1,2})"
+ "([[:space:]]+([Ll][12bxBX]{1,2}))?"
/* followed by optional whitespace */
"[[:space:]]*$";
-static regex_t entries_lvl_line_expr;
+static regex_t entries_line_expr;
+
+enum match_id {
+ FW_TYPE = 1,
+ FW_FILE,
+ OPT_SPACE1,
+ OPT_LEVEL,
+ N_MATCHES,
+};
void compile_reg_expr(int cflags, const char *expr, regex_t *reg)
{
@@ -579,7 +572,6 @@
return OK;
}
-#define N_MATCHES 4
static int is_valid_entry(char *oneline, regmatch_t match[N_MATCHES])
{
int retval, index;
@@ -588,18 +580,14 @@
match[index].rm_so = -1;
match[index].rm_eo = -1;
}
- if (regexec(&entries_line_expr, oneline, 3, match, 0) == 0) {
- oneline[match[1].rm_eo] = '\0';
- oneline[match[2].rm_eo] = '\0';
- retval = 1;
- } else if (regexec(&entries_lvl_line_expr, oneline, 4, match, 0) == 0) {
+ if (regexec(&entries_line_expr, oneline, N_MATCHES, match, 0) == 0) {
/* match[1]: FW type
match[2]: FW filename
- match[3]: Directory level to be dropped
+ match[4]: Optional directory level to be dropped
*/
- oneline[match[1].rm_eo] = '\0';
- oneline[match[2].rm_eo] = '\0';
- oneline[match[3].rm_eo] = '\0';
+ oneline[match[FW_TYPE].rm_eo] = '\0';
+ oneline[match[FW_FILE].rm_eo] = '\0';
+ oneline[match[OPT_LEVEL].rm_eo] = '\0';
retval = 1;
} else {
retval = 0;
@@ -645,10 +633,11 @@
static uint8_t process_one_line(char *oneline, regmatch_t *match, char *dir,
amd_cb_config *cb_config)
{
- char *path_filename, *fn = &(oneline[match[2].rm_so]);
- char *fw_type_str = &(oneline[match[1].rm_so]);
+ char *path_filename, *fn = &(oneline[match[FW_FILE].rm_so]);
+ char *fw_type_str = &(oneline[match[FW_TYPE].rm_so]);
char ch_lvl = 'x';
- regoff_t ch_lvl_index = match[3].rm_so;
+ regoff_t ch_lvl_index = match[OPT_LEVEL].rm_so == match[OPT_LEVEL].rm_eo ?
+ -1 : match[OPT_LEVEL].rm_so;
/* If the optional level field is present,
extract the level char. */
@@ -703,6 +692,8 @@
}
}
+#define FW_LOCATION "FIRMWARE_LOCATION"
+#define SOC_NAME "SOC_NAME"
/*
return value:
0: The config file can not be parsed correctly.
@@ -725,8 +716,6 @@
blank_or_comment_regex, &blank_or_comment_expr);
compile_reg_expr(REG_EXTENDED | REG_NEWLINE,
entries_line_regex, &entries_line_expr);
- compile_reg_expr(REG_EXTENDED | REG_NEWLINE,
- entries_lvl_line_regex, &entries_lvl_line_expr);
/* Get a line */
/* Get FIRMWARE_LOCATION in the first loop */
@@ -735,14 +724,14 @@
if (skip_comment_blank_line(oneline))
continue;
if (is_valid_entry(oneline, match)) {
- if (strcmp(&(oneline[match[1].rm_so]), "FIRMWARE_LOCATION") == 0) {
- dir_len = match[2].rm_eo - match[2].rm_so;
+ if (strcmp(&(oneline[match[FW_TYPE].rm_so]), FW_LOCATION) == 0) {
+ dir_len = match[FW_FILE].rm_eo - match[FW_FILE].rm_so;
assert(dir_len < MAX_LINE_SIZE);
snprintf(dir, MAX_LINE_SIZE, "%.*s", dir_len,
- &(oneline[match[2].rm_so]));
- } else if (strcmp(&(oneline[match[1].rm_so]), "SOC_NAME") == 0) {
- cb_config->soc_id =
- identify_platform(&(oneline[match[2].rm_so]));
+ &(oneline[match[FW_FILE].rm_so]));
+ } else if (strcmp(&(oneline[match[FW_TYPE].rm_so]), SOC_NAME) == 0) {
+ cb_config->soc_id = identify_platform(
+ &(oneline[match[FW_FILE].rm_so]));
}
}
}
@@ -770,8 +759,8 @@
if (skip_comment_blank_line(oneline))
continue;
if (is_valid_entry(oneline, match)) {
- if (strcmp(&(oneline[match[1].rm_so]), "FIRMWARE_LOCATION") == 0 ||
- strcmp(&(oneline[match[1].rm_so]), "SOC_NAME") == 0) {
+ if (strcmp(&(oneline[match[FW_TYPE].rm_so]), FW_LOCATION) == 0 ||
+ strcmp(&(oneline[match[FW_TYPE].rm_so]), SOC_NAME) == 0) {
continue;
} else {
if (process_one_line(oneline, match, dir,