[flashrom] [PATCH 4/4] layout: Better file parsing.

Stefan Tauner stefan.tauner at student.tuwien.ac.at
Thu Sep 12 22:40:08 CEST 2013


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 at 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





More information about the flashrom mailing list