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 <[email protected]> --- 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 _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
