A complete rewrite of the layout file parser:
- safe (unlike before)
- support for comments
- supports including/sourcing of other layout files (relative to the including
file as well as via absolute paths)
- checks for duplicate region names (breaks inclusion loops too)
- can handle region names and layout file names with spaces correctly
- way better diagnostics
- no more character arrays in struct romentry_t but pointers only
Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at>
---
flashrom.8.tmpl | 50 +++++---
layout.c | 348 +++++++++++++++++++++++++++++++++++++++++++++++---------
2 files changed, 331 insertions(+), 67 deletions(-)
diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl
index b1371e4..8793e91 100644
--- a/flashrom.8.tmpl
+++ b/flashrom.8.tmpl
@@ -106,23 +106,46 @@ size for the flash bus.
Read layout entries from
.BR <file> .
.sp
-A layout entry describes an address region of the flash chip and gives it a name. This allows to access certain
-parts of the flash chip only. A layout file can contain multiple entries one per line with the following syntax:
+A layout file is useful for interacting with parts of a flash chip only. It can contain comments which are
+started by a pound sign (#) and causes all following characters on the same line to be ignored with one
+exception: Every layout file
+.B must
+start with a comment in the following form to define the version of the file. Example for the current version 2:
+.sp
+.B " # flashrom layout 2"
+.sp
+Every other line may contain either a command to include another layout file, a layout entry or white space
+(and an optional comment).
+.sp
+To include another file you can use the
+.sp
+.B " source <path>"
+.sp
+syntax where
+.B path
+specifies the absolute or relative path to the file to be included. Relative paths are interpreted to be
+relative to the file containing the include command. If the path contains spaces it has to be written in double
+quotes, or else only the part before the first space will be used.
+.sp
+Each layout entry describes an address region of the flash chip and gives it a name. One entry per line is
+allowed with the following syntax:
.sp
.B " startaddr:endaddr regionname"
.sp
.BR "startaddr " "and " "endaddr "
-are hexadecimal addresses within the ROM file representing the flash ROM contents.
-Please note that using a 0x prefix for those hexadecimal
-numbers is not necessary, but you can't specify decimal/octal numbers.
-.BR "regionname " "is an arbitrary name for the region from "
-.BR "startaddr " "to " "endaddr " "(both addresses included)."
+are addresses within the ROM file representing the flash ROM contents. They are interpreted in the 'usual' form
+i.e.\ a leading 0 means octal, leading 0x or 0X means hexadecimal, everything else is just decimal.
+.BR "regionname " "is the name for the region from " "startaddr " "to " "endaddr " "(both addresses included)."
+If the name contains spaces it has to be written in double quotes, or else only the part before the first space
+will be used.
.sp
Example content of file rom.layout:
.sp
- 00000000:00008fff gfxrom
- 00009000:0003ffff normal
- 00040000:0007ffff fallback
+ # flashrom layout 2
+ source /home/flashrom/include.layout
+ 0x00000000:0x00008fff "gfx rom"
+ 0x00009000:0x0003ffff normal
+ 0x00040000:0x0007ffff fallback
.sp
.TP
.B "\-i, \-\-image <name>[:<file>]"
@@ -136,10 +159,11 @@ parameter specifies the name of an image file that is used to map the contents o
This is useful for example if you want to write only a part of the flash chip and leave everything else alone
without preparing an image of the complete chip manually:
.sp
-If you only want to update the image named
-.BR "normal " "and " "fallback " "in a ROM based on the layout mentioned above, run"
+If you only want to update the regions named
+.BR "normal " "and " "gfx rom " "in a ROM based on the layout mentioned above, run"
.sp
-.B " flashrom \-p prog \-l rom.layout \-i normal -i fallback \-w some.rom"
+
+.B " flashrom \-p prog \-l rom.layout \-i normal -i ""gfx rom"" \-w some.rom"
.sp
.RB "Overlapping sections are resolved in an implementation-dependent manner - do " "not " "rely on it."
.TP
diff --git a/layout.c b/layout.c
index 20fe303..5b4bee4 100644
--- a/layout.c
+++ b/layout.c
@@ -22,9 +22,11 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <ctype.h>
#include <limits.h>
#include <errno.h>
#ifndef __LIBPAYLOAD__
+#include <libgen.h>
#include <fcntl.h>
#include <sys/stat.h>
#endif
@@ -32,12 +34,15 @@
#include "programmer.h"
#define MAX_ROMLAYOUT 32
+#define MAX_ENTRY_LEN 1024
+#define WHITESPACE_CHARS " \t"
+#define INCLUDE_INSTR "source"
typedef struct {
unsigned int start;
unsigned int end;
unsigned int included;
- char name[256];
+ char *name;
char *file;
} romentry_t;
@@ -50,59 +55,307 @@ static int num_rom_entries = 0; /* the number of valid rom_entries */
static char *include_args[MAX_ROMLAYOUT];
static int num_include_args = 0; /* the number of valid include_args. */
-#ifndef __LIBPAYLOAD__
-int read_romlayout(char *name)
+/* returns the index of the entry (or a negative value if it is not found) */
+static int find_romentry(char *name)
{
- FILE *romlayout;
- char tempstr[256];
int i;
+ msg_gspew("Looking for region \"%s\"... ", name);
+ for (i = 0; i < num_rom_entries; i++) {
+ if (strcmp(rom_entries[i].name, name) == 0) {
+ msg_gspew("found.\n");
+ return i;
+ }
+ }
+ msg_gspew("not found.\n");
+ return -1;
+}
- romlayout = fopen(name, "r");
+/* FIXME: While libpayload has no file I/O itself, code using libflashrom could still provide layout information
+ * obtained by other means like user input or by fetching it from somewhere else. Therefore the parsing code
+ * should be separated from the file reading code eventually. */
+#ifndef __LIBPAYLOAD__
+/** Parse a \em possibly quoted string.
+ *
+ * The function expects \a startp to point to the string to be parsed without leading white space. If the
+ * string starts with a quotation mark it looks for the second one and removes both in-place, if not then it
+ * looks for the first white space character and uses that as delimiter of the wanted string.
+ * After returning \a startp will point to a string that is either the original string with the quotation marks
+ * removed, or the first word of that string before a white space.
+ * If \a endp is not NULL it will be set to point to the first character after the parsed string, which might
+ * well be the '\0' at the end of the string pointed to by \a startp if there are no more characters.
+ *
+ * @param start Points to the input string.
+ * @param end Is set to the first char following the input string.
+ * @return 0 on success, 1 if the parsed string is empty or a quotation mark is not matched.
+ */
+static int unquote_string(char **startp, char **endp)
+{
+ char *end;
+ size_t len;
+ if (**startp == '"') {
+ (*startp)++;
+ len = strcspn(*startp, "\"");
+ } else {
+ len = strcspn(*startp, WHITESPACE_CHARS);
+ }
+ if (len == 0)
+ return 1;
- if (!romlayout) {
- msg_gerr("ERROR: Could not open ROM layout (%s).\n",
- name);
- return -1;
+ end = *startp + len;
+ if (*end != '\0') {
+ *end = '\0';
+ end++;
}
+ if (endp != NULL)
+ *endp = end;
+ msg_gspew("%s: start=\"%s\", end=\"%s\"\n", __func__, *startp, end);
+ return 0;
+}
- while (!feof(romlayout)) {
- char *tstr1, *tstr2;
+/** Parse one line in a layout file.
+ * @param file_name The name of the file this line originates from.
+ * @param file_version The version of the layout specification to be used.
+ * @param entry If not NULL fill it with the parsed data, else just detect errors and print diagnostics.
+ * @return 1 on error,
+ * 0 if the line could be parsed into a layout entry succesfully,
+ * -1 if a file was successfully sourced.
+ */
+static int parse_entry(char *file_name, int file_version, char *buf, romentry_t *entry)
+{
+ int addr_base;
+ if (file_version == 1)
+ addr_base = 16;
+ else if (file_version >= 2)
+ addr_base = 0; /* autodetect */
+ else
+ return 1;
- if (num_rom_entries >= MAX_ROMLAYOUT) {
- msg_gerr("Maximum number of ROM images (%i) in layout "
- "file reached.\n", MAX_ROMLAYOUT);
- return 1;
- }
- if (2 != fscanf(romlayout, "%s %s\n", tempstr, rom_entries[num_rom_entries].name))
- continue;
-#if 0
- // fscanf does not like arbitrary comments like that :( later
- if (tempstr[0] == '#') {
- continue;
+ msg_gdbg2("String to parse: \"%s\".\n", buf);
+
+ /* Skip all white space in the beginning. */
+ char *tmp_str = buf + strspn(buf, WHITESPACE_CHARS);
+ char *endptr;
+
+ /* Check for include command. */
+ if (file_version >= 2 && strncmp(tmp_str, INCLUDE_INSTR, strlen(INCLUDE_INSTR)) == 0) {
+ tmp_str += strlen(INCLUDE_INSTR);
+ tmp_str += strspn(tmp_str, WHITESPACE_CHARS);
+ if (unquote_string(&tmp_str, NULL) != 0) {
+ msg_gerr("Error parsing version %d layout entry: Could not find file name in \"%s\".\n",
+ file_version, buf);
+ return 1;
}
-#endif
- tstr1 = strtok(tempstr, ":");
- tstr2 = strtok(NULL, ":");
- if (!tstr1 || !tstr2) {
- msg_gerr("Error parsing layout file. Offending string: \"%s\"\n", tempstr);
- fclose(romlayout);
+ msg_gspew("Source command found with filename \"%s\".\n", tmp_str);
+
+ int ret;
+ /* If a relative path is given, append it to the dirname of the current file. */
+ if (*tmp_str != '/') {
+ /* We need space for: dirname of file_name, '/' , the file name in tmp_strand and '\0'.
+ * Since the dirname of file_name is shorter than file_name this is more than enough: */
+ char *path = malloc(strlen(file_name) + strlen(tmp_str) + 2);
+ if (path == NULL) {
+ msg_gerr("Out of memory!\n");
+ return 1;
+ }
+ strcpy(path, file_name);
+
+ /* A less insane but incomplete dirname implementation... */
+ endptr = strrchr(path, '/');
+ if (endptr != NULL) {
+ endptr[0] = '/';
+ endptr[1] = '\0';
+ } else {
+ /* This is safe because the original file name was at least one char. */
+ path[0] = '.';
+ path[1] = '/';
+ path[2] = '\0';
+ }
+ strcat(path, tmp_str);
+ ret = read_romlayout(path);
+ free(path);
+ } else
+ ret = read_romlayout(tmp_str);
+ return ret == 0 ? -1 : 1;
+ }
+
+ errno = 0;
+ long tmp_long = strtol(tmp_str, &endptr, addr_base);
+ if (errno != 0 || endptr == tmp_str || tmp_long < 0 || tmp_long > UINT32_MAX) {
+ msg_gerr("Error parsing version %d layout entry: Could not convert start address in \"%s\".\n",
+ file_version, buf);
+ return 1;
+ }
+ uint32_t start = tmp_long;
+
+ tmp_str = endptr + strspn(endptr, WHITESPACE_CHARS);
+ if (*tmp_str != ':') {
+ msg_gerr("Error parsing version %d layout entry: Address separator does not follow start address in \"%s\""
+ ".\n", file_version, buf);
+ return 1;
+ }
+ tmp_str++;
+
+ errno = 0;
+ tmp_long = strtol(tmp_str, &endptr, addr_base);
+ if (errno != 0 || endptr == tmp_str || tmp_long < 0 || tmp_long > UINT32_MAX) {
+ msg_gerr("Error parsing version %d layout entry: Could not convert end address in \"%s\"\n",
+ file_version, buf);
+ return 1;
+ }
+ uint32_t end = tmp_long;
+
+ size_t skip = strspn(endptr, WHITESPACE_CHARS);
+ if (skip == 0) {
+ msg_gerr("Error parsing version %d layout entry: End address is not followed by white space in "
+ "\"%s\"\n", file_version, buf);
+ return 1;
+ }
+
+ tmp_str = endptr + skip;
+ /* The region name is either enclosed by quotes or ends with the first whitespace. */
+ if (unquote_string(&tmp_str, &endptr) != 0) {
+ msg_gerr("Error parsing version %d layout entry: Could not find region name in \"%s\".\n",
+ file_version, buf);
+ return 1;
+ }
+
+ msg_gdbg("Parsed entry: 0x%08x - 0x%08x named \"%s\"\n", start, end, tmp_str);
+
+ if (start >= end) {
+ msg_gerr("Error parsing version %d layout entry: Length of region \"%s\" is not positive.\n",
+ file_version, tmp_str);
+ return 1;
+ }
+
+ if (find_romentry(tmp_str) >= 0) {
+ msg_gerr("Error parsing version %d layout entry: Region name \"%s\" used multiple times.\n",
+ file_version, tmp_str);
+ return 1;
+ }
+
+ endptr += strspn(endptr, WHITESPACE_CHARS);
+ if (strlen(endptr) != 0)
+ msg_gerr("Warning: Region name \"%s\" is not followed by white space only.\n", tmp_str);
+
+ if (entry != NULL) {
+ entry->name = strdup(tmp_str);
+ if (entry->name == NULL) {
+ msg_gerr("Out of memory!\n");
return 1;
}
- rom_entries[num_rom_entries].start = strtol(tstr1, (char **)NULL, 16);
- rom_entries[num_rom_entries].end = strtol(tstr2, (char **)NULL, 16);
- rom_entries[num_rom_entries].included = 0;
- rom_entries[num_rom_entries].file = NULL;
- num_rom_entries++;
+
+ entry->start = start;
+ entry->end = end;
+ entry->included = 0;
+ entry->file = NULL;
}
+ return 0;
+}
- for (i = 0; i < num_rom_entries; i++) {
- msg_gdbg("romlayout %08x - %08x named %s\n",
- rom_entries[i].start,
- rom_entries[i].end, rom_entries[i].name);
+/* Scan the first line for the determinant version comment and parse it, or assume it is version 1. */
+static int detect_layout_version(FILE *romlayout)
+{
+ int c;
+ do { /* Skip white space */
+ c = fgetc(romlayout);
+ if (c == EOF)
+ return -1;
+ } while (isblank(c));
+ ungetc(c, romlayout);
+
+ const char* vcomment = "# flashrom layout ";
+ char buf[strlen(vcomment) + 1]; /* comment + \0 */
+ if (fgets(buf, sizeof(buf), romlayout) == NULL)
+ return -1;
+ if (strcmp(vcomment, buf) != 0)
+ return 1;
+ int version;
+ if (fscanf(romlayout, "%d", &version) != 1)
+ return -1;
+ if (version < 2) {
+ msg_gwarn("Warning: Layout file declares itself to be version %d, but self delcaration has\n"
+ "only been possible since version 2. Continuing anyway.\n", version);
}
+ return version;
+}
- fclose(romlayout);
+int read_romlayout(char *name)
+{
+ FILE *romlayout = fopen(name, "r");
+ if (romlayout == NULL) {
+ msg_gerr("ERROR: Could not open layout file \"%s\".\n", name);
+ return -1;
+ }
+
+ int file_version = detect_layout_version(romlayout);
+ if (file_version < 0) {
+ msg_gerr("Could not determine version of layout file \"%s\".\n", name);
+ fclose(romlayout);
+ return 1;
+ }
+ if (file_version < 1 || file_version > 2) {
+ msg_gerr("Unknown layout file version: %d\n", file_version);
+ fclose(romlayout);
+ return 1;
+ }
+ rewind(romlayout);
+
+ msg_gdbg("Parsing layout file \"%s\" according to version %d.\n", name, file_version);
+ int linecnt = 0;
+ while (!feof(romlayout)) {
+ char buf[MAX_ENTRY_LEN];
+ char *curchar = buf;
+ linecnt++;
+ msg_gspew("Parsing line %d of \"%s\".\n", linecnt, name);
+
+ while (true) {
+ char c = fgetc(romlayout);
+ if (c == '#') {
+ if (file_version == 1) {
+ msg_gerr("Line %d of version %d layout file \"%s\" contains a "
+ "forbidden #.\n", linecnt, file_version, name);
+ fclose(romlayout);
+ return 1;
+ }
+ do { /* Skip characters in comments */
+ c = fgetc(romlayout);
+ } while (c != EOF && c != '\n');
+ continue;
+ }
+ if (c == EOF || c == '\n') {
+ *curchar = '\0';
+ break;
+ }
+ if (curchar == &buf[MAX_ENTRY_LEN - 1]) {
+ msg_gerr("Line %d of layout file \"%s\" is longer than the allowed %d chars.\n",
+ linecnt, name, MAX_ENTRY_LEN);
+ fclose(romlayout);
+ return 1;
+ }
+ *curchar = c;
+ curchar++;
+ }
+
+ /* Skip all whitespace or empty lines */
+ if (strspn(buf, WHITESPACE_CHARS) == strlen(buf))
+ continue;
+ romentry_t *entry = (num_rom_entries >= MAX_ROMLAYOUT) ? NULL : &rom_entries[num_rom_entries];
+ int ret = parse_entry(name, file_version, buf, entry);
+ if (ret > 0) {
+ fclose(romlayout);
+ return 1;
+ }
+ /* Only 0 indicates the successfully parsing of a layout entry, -1 indicates a sourced file. */
+ if (ret == 0)
+ num_rom_entries++;
+ }
+ fclose(romlayout);
+ if (num_rom_entries >= MAX_ROMLAYOUT) {
+ msg_gerr("Found %d entries in layout file which is more than the %i allowed.\n",
+ num_rom_entries + 1, MAX_ROMLAYOUT);
+ return 1;
+ }
return 0;
}
#endif
@@ -141,21 +394,6 @@ int register_include_arg(char *name)
return 0;
}
-/* returns the index of the entry (or a negative value if it is not found) */
-static int find_romentry(char *name)
-{
- int i;
- msg_gspew("Looking for region \"%s\"... ", name);
- for (i = 0; i < num_rom_entries; i++) {
- if (strcmp(rom_entries[i].name, name) == 0) {
- msg_gspew("found.\n");
- return i;
- }
- }
- msg_gspew("not found.\n");
- return -1;
-}
-
/* process -i arguments
* returns 0 to indicate success, >0 to indicate failure
*/
@@ -219,6 +457,8 @@ void layout_cleanup(void)
num_include_args = 0;
for (i = 0; i < num_rom_entries; i++) {
+ free(rom_entries[i].name);
+ rom_entries[i].name = NULL;
free(rom_entries[i].file);
rom_entries[i].file = NULL;
rom_entries[i].included = 0;
--
Kind regards, Stefan Tauner
