Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42859 )
Change subject: amdfwtool: Take a config file instead of command line parameters ......................................................................
Patch Set 54:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42859/54/util/amdfwtool/data_parse.... File util/amdfwtool/data_parse.c:
https://review.coreboot.org/c/coreboot/+/42859/54/util/amdfwtool/data_parse.... PS54, Line 265: (void *) this cast is probably unnecessary, since both sides are amd_bios_entry type
https://review.coreboot.org/c/coreboot/+/42859/54/util/amdfwtool/data_parse.... PS54, Line 317: /* get line */ : /* blank comment */ : if (!regexec(&blank_or_comment_expr, oneline, 0, NULL, 0)) { : /* skip comment and blank */ : continue; : } : : if (regexec(&entries_line_expr, oneline, 3, match, 0) == 0) { : oneline[match[1].rm_eo] = '\0'; : oneline[match[2].rm_eo] = '\0'; : } else { : /* no match */ : continue; : } this code is identical to lines 340-353 below, so it's probably a good idea to factor that out into a function (foe example is_valid_config_line(...)) that returns a boolean and then either run the different code parts inside the two loops or continue to the next line