[U-Boot] [PATCH 04/11] imximage: cleanup parsing

Troy Kisky troy.kisky at boundarydevices.com
Wed Sep 19 02:03:02 CEST 2012


Move to pulling tokens instead of pushing them.
Remove need for switch statements to process commands.
Add error messages such as "command not finished",
"extra data at end of line", and "invalid token"
Add ';' as command separator.

Signed-off-by: Troy Kisky <troy.kisky at boundarydevices.com>
---
 tools/imximage.c |  380 ++++++++++++++++++++++++++++++------------------------
 tools/imximage.h |   25 ++--
 2 files changed, 226 insertions(+), 179 deletions(-)

diff --git a/tools/imximage.c b/tools/imximage.c
index 21c49e6..1e120354 100644
--- a/tools/imximage.c
+++ b/tools/imximage.c
@@ -75,21 +75,6 @@ static uint32_t g_flash_offset;
 
 static struct image_type_params imximage_params;
 
-static uint32_t get_cfg_value(char *token, char *name,  int linenr)
-{
-	char *endptr;
-	uint32_t value;
-
-	errno = 0;
-	value = strtoul(token, &endptr, 16);
-	if (errno || (token == endptr)) {
-		fprintf(stderr, "Error: %s[%d] - Invalid hex data(%s)\n",
-			name,  linenr, token);
-		exit(EXIT_FAILURE);
-	}
-	return value;
-}
-
 static uint32_t detect_imximage_version(struct imx_header *imx_hdr)
 {
 	imx_header_v1_t *hdr_v1 = &imx_hdr->header.hdr_v1;
@@ -120,55 +105,38 @@ static void err_imximage_version(int version)
 
 static uint32_t *p_entry;
 
-static void set_dcd_val_v1(struct imx_header *imxhdr, char *name, int lineno,
-					int fld, uint32_t value)
+static int set_dcd_val_v1(struct imx_header *imxhdr, uint32_t *data)
 {
 	dcd_v1_t *dcd_v1 = &imxhdr->header.hdr_v1.dcd_table;
+	uint32_t val = *data++;
 
-	switch (fld) {
-	case CFG_REG_SIZE:
-		/* Byte, halfword, word */
-		if ((value != 1) && (value != 2) && (value != 4)) {
-			fprintf(stderr, "Error: %s[%d] - "
-				"Invalid register size " "(%d)\n",
-				name, lineno, value);
-			exit(EXIT_FAILURE);
-		}
-		*p_entry++ = value;
-		break;
-	case CFG_REG_ADDRESS:
-		*p_entry++ = value;
-		break;
-	case CFG_REG_VALUE:
-		*p_entry++ = value;
-		dcd_v1->preamble.length = (char *)p_entry
-				- (char *)&dcd_v1->addr_data[0].type;
-		break;
-	default:
-		break;
-
+	/* Byte, halfword, word */
+	if ((val != 1) && (val != 2) && (val != 4)) {
+		fprintf(stderr, "Error: Invalid register size (%d)\n", val);
+		return -1;
 	}
+	*p_entry++ = val;
+	*p_entry++ = *data++;
+	*p_entry++ = *data++;
+	dcd_v1->preamble.length = (char *)p_entry - (char *)&dcd_v1->
+			addr_data[0].type;
+	return 0;
 }
 
 static write_dcd_command_t *p_dcd;
 
-static void set_dcd_val_v2(struct imx_header *imxhdr, char *name, int lineno,
-					int fld, uint32_t value)
+static int set_dcd_val_v2(struct imx_header *imxhdr, uint32_t *data)
 {
 	uint32_t len;
 	dcd_v2_t *dcd_v2 = &imxhdr->header.hdr_v2.dcd_table;
+	uint32_t val = *data++;
 
-	switch (fld) {
-	case CFG_REG_SIZE:
-		/* Byte, halfword, word */
-		if ((value != 1) && (value != 2) && (value != 4)) {
-			fprintf(stderr, "Error: %s[%d] - "
-				"Invalid register size " "(%d)\n",
-				name, lineno, value);
-			exit(EXIT_FAILURE);
-		}
-		if (p_dcd && (p_dcd->param == value))
-			break;
+	/* Byte, halfword, word */
+	if ((val != 1) && (val != 2) && (val != 4)) {
+		fprintf(stderr, "Error: Invalid register size (%d)\n", val);
+		return -1;
+	}
+	if (!(p_dcd && (p_dcd->param == val))) {
 		if (!p_dcd) {
 			dcd_v2->header.tag = DCD_HEADER_TAG;
 			dcd_v2->header.version = DCD_VERSION;
@@ -176,24 +144,19 @@ static void set_dcd_val_v2(struct imx_header *imxhdr, char *name, int lineno,
 		} else {
 			p_dcd = (write_dcd_command_t *)p_entry;
 		}
-		p_dcd->param = value;
+		p_dcd->param = val;
 		p_dcd->tag = DCD_COMMAND_TAG;
 		p_entry = (uint32_t *)(p_dcd + 1);
-		break;
-	case CFG_REG_ADDRESS:
-		*p_entry++ = cpu_to_be32(value);
-		break;
-	case CFG_REG_VALUE:
-		*p_entry++ = cpu_to_be32(value);
-		len = (char *)p_entry - (char *)&dcd_v2->header;
-		dcd_v2->header.length = cpu_to_be16(len);
-		len = (char *)p_entry - (char *)p_dcd;
-		p_dcd->length = cpu_to_be16(len);
-		break;
-	default:
-		break;
-
 	}
+	val = *data++;
+	*p_entry++ = cpu_to_be32(val);
+	val = *data++;
+	*p_entry++ = cpu_to_be32(val);
+	len = (char *)p_entry - (char *)&dcd_v2->header;
+	dcd_v2->header.length = cpu_to_be16(len);
+	len = (char *)p_entry - (char *)p_dcd;
+	p_dcd->length = cpu_to_be16(len);
+	return 0;
 }
 
 static int set_imx_hdr_v1(struct imx_header *imxhdr,
@@ -347,93 +310,186 @@ static void print_hdr_v2(struct imx_header *imx_hdr)
 	printf("Entry Point:  %08x\n", (uint32_t)fhdr_v2->entry);
 }
 
-static void parse_cfg_cmd(struct imx_header *imxhdr, int32_t cmd, char *token,
-				char *name, int lineno, int fld)
+static int cmd_cnt;
+
+int skip_separators(struct data_src *ds)
 {
-	int value;
-	static int cmd_ver_first = ~0;
-
-	switch (cmd) {
-	case CMD_IMAGE_VERSION:
-		imximage_version = get_cfg_value(token, name, lineno);
-		if (cmd_ver_first == 0) {
-			fprintf(stderr, "Error: %s[%d] - IMAGE_VERSION "
-				"command need be the first before other "
-				"valid command in the file\n", name, lineno);
-			exit(EXIT_FAILURE);
+	int line_no = ds->lineno;
+	char *p = ds->p;
+
+	for (;;) {
+		char c;
+		if (!p) {
+			if (getline(&ds->line, &ds->len, ds->fd) <= 0)
+				return -1;
+			ds->lineno++;
+			p = ds->line;
+			if (ds->cmd_started) {
+				fprintf(stderr, "warning: continuing command on"
+						" next line, line %s[%d](%s)\n",
+						ds->filename, ds->lineno, p);
+			}
 		}
-		cmd_ver_first = 1;
-		set_hdr_func(imxhdr);
-		break;
-	case CMD_BOOT_FROM:
-		g_flash_offset = get_table_entry_id(imximage_bootops,
-					"imximage boot option", token);
-		if (g_flash_offset == -1) {
-			fprintf(stderr, "Error: %s[%d] -Invalid boot device"
-				"(%s)\n", name, lineno, token);
-			exit(EXIT_FAILURE);
+		c = *p;
+		if ((c == ' ') || (c == '\t')) {
+			p++;
+			continue;
 		}
-		if (unlikely(cmd_ver_first != 1))
-			cmd_ver_first = 0;
-		break;
-	case CMD_DATA:
-		value = get_cfg_value(token, name, lineno);
-		(*set_dcd_val)(imxhdr, name, lineno, fld, value);
-		if (unlikely(cmd_ver_first != 1))
-			cmd_ver_first = 0;
-		break;
+		/* Drop all text starting with '#' as comments */
+		if ((c == '#') || (c == '\r') || (c == '\n')
+				|| !c) {
+			p = NULL;
+			continue;
+		}
+		if (c == ';') {
+			if (ds->cmd_started) {
+				fprintf(stderr, "Error: command not "
+						"finished:%s[%d](%s)\n",
+						ds->filename, ds->lineno, p);
+				exit(EXIT_FAILURE);
+			}
+			p++;
+			continue;
+		}
+		if (!ds->cmd_started && line_no == ds->lineno) {
+			fprintf(stderr, "warning: extra data at end "
+					"of line %s[%d](%s)\n",
+					ds->filename, ds->lineno, p);
+			p = NULL;
+			continue;
+		}
+		ds->p = p;
+		return 0;
 	}
 }
 
-static void parse_cfg_fld(struct imx_header *imxhdr, int32_t *cmd,
-		char *token, char *name, int lineno, int fld)
+char *grab_token(char *dest, int size, char *src)
 {
-	int value;
-
-	switch (fld) {
-	case CFG_COMMAND:
-		*cmd = get_table_entry_id(imximage_cmds,
-			"imximage commands", token);
-		if (*cmd < 0) {
-			fprintf(stderr, "Error: %s[%d] - Invalid command"
-			"(%s)\n", name, lineno, token);
-			exit(EXIT_FAILURE);
-		}
-		break;
-	case CFG_REG_SIZE:
-		parse_cfg_cmd(imxhdr, *cmd, token, name, lineno, fld);
-		break;
-	case CFG_REG_ADDRESS:
-	case CFG_REG_VALUE:
-		if (*cmd != CMD_DATA)
-			return;
-
-		value = get_cfg_value(token, name, lineno);
-		(*set_dcd_val)(imxhdr, name, lineno, fld, value);
-		if (p_entry > p_max_dcd) {
-			uint32_t size = (char *)p_max_dcd - (char *)imxhdr;
-			fprintf(stderr, "Error: %s[%d] -"
-					"header exceeds maximum size(%d)\n",
-					name, lineno, size);
-			exit(EXIT_FAILURE);
-		}
-		break;
-	default:
-		break;
+	while (size) {
+		char c = *src;
+		if ((c == ' ') || (c == '\t') || (c == '\r') || (c == '\n')
+				|| (c == '#') || !c)
+			break;
+		*dest++ = c;
+		size--;
+		src++;
+	}
+	if (!size)
+		return NULL;
+	*dest = 0;
+	return src;
+}
+
+static uint32_t get_cfg_value(struct data_src *ds, uint32_t *pval)
+{
+	char *endptr;
+	uint32_t value;
+
+	if (skip_separators(ds))
+		return -1;
+	errno = 0;
+	value = strtoul(ds->p, &endptr, 16);
+	if (errno || (ds->p == endptr))
+		return -1;
+	*pval = value;
+	ds->p = endptr;
+	return 0;
+}
+
+static int parse_cmd_data(struct data_src *ds)
+{
+	uint32_t data[3];
+	int ret = get_cfg_value(ds, &data[0]);
+
+	if (ret)
+		return ret;
+	ret = get_cfg_value(ds, &data[1]);
+	if (ret)
+		return ret;
+	ret = get_cfg_value(ds, &data[2]);
+	if (ret)
+		return ret;
+	ret = (*set_dcd_val)(ds->imxhdr, data);
+	if (ret)
+		return ret;
+	if (p_entry > p_max_dcd) {
+		uint32_t size = (char *)p_max_dcd - (char *)ds->imxhdr;
+		fprintf(stderr, "Error: header exceeds maximum size(%d)\n",
+				size);
+		return -1;
+	}
+	return 0;
+}
+
+static int parse_image_version(struct data_src *ds)
+{
+	int ret;
+
+	ret = get_cfg_value(ds, &imximage_version);
+	if (ret)
+		return ret;
+	if (cmd_cnt) {
+		fprintf(stderr, "Error: IMAGE_VERSION command needs be "
+				"before other valid commands in the file\n");
+		return -1;
+	}
+	set_hdr_func(ds->imxhdr);
+	return 0;
+}
+
+int get_from_array(struct data_src *ds,
+		const table_entry_t *table, const char *table_name)
+{
+	int val;
+	char token[16];
+	char *p;
+
+	if (skip_separators(ds))
+		return -1;
+	p = grab_token(token, sizeof(token), ds->p);
+	if (!p)
+		return -1;
+	val = get_table_entry_id(table, table_name, token);
+	if (val != -1)
+		ds->p = p;
+	return val;
+}
+
+static int parse_boot_from(struct data_src *ds)
+{
+	g_flash_offset = get_from_array(ds, imximage_bootops,
+			"imximage boot option");
+	if (g_flash_offset == -1) {
+		fprintf(stderr, "Error: Invalid boot device\n");
+		return -1;
 	}
+	return 0;
 }
+
+parse_fld_t cmd_table[] = {
+	parse_image_version, parse_boot_from, parse_cmd_data
+};
+
+static int parse_command(struct data_src *ds)
+{
+	int cmd = get_from_array(ds, imximage_cmds, "imximage commands");
+	if (cmd < 0)
+		return cmd;
+	return cmd_table[cmd](ds);
+}
+
 static void parse_cfg_file(struct imx_header *imxhdr, char *name)
 {
-	FILE *fd = NULL;
-	char *line = NULL;
-	char *token, *saveptr1, *saveptr2;
-	int lineno = 0;
-	int fld;
-	size_t len;
-	int32_t cmd;
-
-	fd = fopen(name, "r");
-	if (fd == 0) {
+	struct data_src ds;
+
+	ds.line = NULL;
+	ds.len = 0;
+	ds.lineno = 0;
+	ds.filename = name;
+	ds.fd = fopen(name, "r");
+	ds.imxhdr = imxhdr;
+	ds.p = NULL;
+	if (ds.fd == 0) {
 		fprintf(stderr, "Error: %s - Can't open DCD file\n", name);
 		exit(EXIT_FAILURE);
 	}
@@ -441,34 +497,22 @@ static void parse_cfg_file(struct imx_header *imxhdr, char *name)
 	/* Very simple parsing, line starting with # are comments
 	 * and are dropped
 	 */
-	while ((getline(&line, &len, fd)) > 0) {
-		lineno++;
-
-		token = strtok_r(line, "\r\n", &saveptr1);
-		if (token == NULL)
-			continue;
-
-		/* Check inside the single line */
-		for (fld = CFG_COMMAND, cmd = CMD_INVALID,
-				line = token; ; line = NULL, fld++) {
-			token = strtok_r(line, " \t", &saveptr2);
-			if (token == NULL)
-				break;
-
-			/* Drop all text starting with '#' as comments */
-			if (token[0] == '#')
-				break;
-
-			parse_cfg_fld(imxhdr, &cmd, token, name,
-					lineno, fld);
+	for (;;) {
+		ds.cmd_started = 0;
+		if (skip_separators(&ds))
+			break;
+		ds.cmd_started = 1;
+		if (parse_command(&ds)) {
+			fprintf(stderr, "Error: invalid token "
+					"%s[%d](%s)\n", name, ds.lineno, ds.p);
+			exit(EXIT_FAILURE);
 		}
-
+		cmd_cnt++;
 	}
-	fclose(fd);
+	fclose(ds.fd);
 	return;
 }
 
-
 static int imximage_check_image_types(uint8_t type)
 {
 	if (type == IH_TYPE_IMXIMAGE)
@@ -557,12 +601,12 @@ static void imximage_set_header(void *ptr, struct stat *sbuf, int ifd,
 int imximage_check_params(struct mkimage_params *params)
 {
 	if (!params)
-		return CFG_INVALID;
+		return -1;
 	if (!strlen(params->imagename)) {
 		fprintf(stderr, "Error: %s - Configuration file not specified, "
 			"it is needed for imximage generation\n",
 			params->cmdname);
-		return CFG_INVALID;
+		return -1;
 	}
 	/*
 	 * Check parameters:
diff --git a/tools/imximage.h b/tools/imximage.h
index 0319c02..efd249b 100644
--- a/tools/imximage.h
+++ b/tools/imximage.h
@@ -49,20 +49,11 @@
 #define DCD_VERSION 0x40
 
 enum imximage_cmd {
-	CMD_INVALID,
 	CMD_IMAGE_VERSION,
 	CMD_BOOT_FROM,
 	CMD_DATA
 };
 
-enum imximage_fld_types {
-	CFG_INVALID = -1,
-	CFG_COMMAND,
-	CFG_REG_SIZE,
-	CFG_REG_ADDRESS,
-	CFG_REG_VALUE
-};
-
 enum imximage_version {
 	IMXIMAGE_VER_INVALID = -1,
 	IMXIMAGE_V1 = 1,
@@ -158,8 +149,20 @@ struct imx_header {
 	} header;
 };
 
-typedef void (*set_dcd_val_t)(struct imx_header *imxhdr, char *name,
-		int lineno, int fld, uint32_t value);
+struct data_src {
+	char *line;
+	size_t len;
+	FILE *fd;
+	int lineno;
+	char cmd_started;
+	char *filename;
+	struct imx_header *imxhdr;
+	char *p;
+};
+
+typedef int (*parse_fld_t)(struct data_src *ds);
+
+typedef int (*set_dcd_val_t)(struct imx_header *imxhdr, uint32_t *data);
 
 typedef int (*set_imx_hdr_t)(struct imx_header *imxhdr, uint32_t entry_point,
 		uint32_t flash_offset);
-- 
1.7.9.5



More information about the U-Boot mailing list