Change in flashrom[master]: Match -c parameter against patterns in chip names

[Ryan O'Leary (Code Review)]

Ryan O'Leary has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37723 )

Change subject: Match -c parameter against patterns in chip names
......................................................................

Match -c parameter against patterns in chip names

This includes patterns such as "EN29GL064H/L", "Am29F002(N)BB" and
"A25LQ032/A25LQ32A".

This was inspired by a breakage in one of my scripts. The script
hard-coded "-c CHIPX". When upstream changed the chip name to
"CHIPX/CHIPY", my script broke. By recognizing this pattern, the -c
parameter is more resilient to these types of changes.

Since an exact match on the whole name takes priority (the current
behavior), this should not break anyone's workflow.

Change-Id: Iae00bb1108f2d5a0c10977cff63f9348b9fc017f
Signed-off-by: Ryan O'Leary 
---
M cli_classic.c
1 file changed, 69 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/37723/1

diff --git a/cli_classic.c b/cli_classic.c
index 73cc417..43aac0b 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -103,6 +103,68 @@
    return 0;
 }

+/*
+ * Return 1 if the chip name matches the pattern, otherwise 0.
+ *
+ * The pattern may be:
+ * 1. A single slash followed by a single character.
+ *    Ex: "EN29GL064H/L" matches "EN29GL064H" and "EN29GL064L".
+ * 2. One character in parens.
+ *    Ex: "Am29F002(N)BB" matches "Am29F002BB" and "Am29F002NBB".
+ *    Ex: "EN29F002(A)(N)B" matches four different chip names.
+ * 3. A slash-separated list.
+ *    Ex: "A25LQ032/A25LQ32A" matches "A25LQ032" and "A25LQ32A".
+ *    Ex: "MX25L12835F/MX25L12845E/MX25L12865E" matches "MX25L12835F", "MX25L12845E" and "MX25L12865E".
+ *
+ * #2 may be nested in #3, for example "W29C010(M)/W29C011A/W29EE011/W29EE012".
+ */
+static int chip_name_match_pattern(const char *pattern, const char *chip) {
+	int plen = strlen(pattern);
+	int clen = strlen(chip);
+
+	if (plen < 3 || clen < 3) {
+		return 0;
+	}
+
+	/* Single slash followed by single character. */
+	if (pattern[plen-2] == '/') {
+		for (int pidx = 0; pidx < plen - 2; pidx++)
+			if (pattern[pidx] == '/')
+				return 0;
+		return plen - 2 == clen &&
+		    strncmp(pattern, chip, plen - 3) == 0 &&
+		    (pattern[plen-1] == chip[clen-1] ||
+		     pattern[plen-3] == chip[clen-1]);
+	}
+
+	int cidx = 0;
+	for (int pidx = 0; pidx < plen; pidx++) {
+		if (pattern[pidx] == '/') {
+			/* Check for termination. */
+			if (cidx == clen)
+				break;
+			cidx = 0; /* Reset. */
+		} else if (pattern[pidx] == '(') {
+			if (pidx + 2 >= plen ||
+			    pattern[pidx+1] == ')' ||
+			    pattern[pidx+1] == '(' ||
+			    pattern[pidx+2] != ')')
+				return 0; /* Invalid pattern. */
+			if (pattern[pidx+1] == chip[cidx])
+				cidx++;
+			pidx += 2;
+		} else if (pattern[pidx] == chip[cidx]) {
+			cidx++;
+		} else {
+			/* Reset to next slash. */
+			while (pidx < plen && pattern[pidx] != '/')
+				pidx++;
+			cidx = 0;
+		}
+	}
+	return cidx == clen;
+}
+
 int main(int argc, char *argv[])
 {
    const struct flashchip *chip = NULL;
@@ -421,15 +483,22 @@
    }
    /* Does a chip with the requested name exist in the flashchips array? */
    if (chip_to_probe) {
+		/* First check for exact match. */
    	for (chip = flashchips; chip && chip->name; chip++)
    		if (!strcmp(chip->name, chip_to_probe))
    			break;
+		/* Failing that, perform a pattern match. */
+		if (!chip || !chip->name)
+			for (chip = flashchips; chip && chip->name; chip++)
+				if (chip_name_match_pattern(chip->name, chip_to_probe))
+					break;
    	if (!chip || !chip->name) {
    		msg_cerr("Error: Unknown chip '%s' specified.\n", chip_to_probe);
    		msg_gerr("Run flashrom -L to view the hardware supported in this flashrom version.\n");
    		ret = 1;
    		goto out;
    	}
+		msg_gdbg("Using chip "%s".\n", chip->name);
    	/* Keep chip around for later usage in case a forced read is requested. */
    }

[Carl-Daniel Hailfinger (Code Review)]

Carl-Daniel Hailfinger has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37723 )

Change subject: Match -c parameter against patterns in chip names
......................................................................

Patch Set 1:

This is a cool idea and it will help in some cases.
In the past, we had some chip definitions which conflicted with such an approach, though: There was a generic chip entry for one family of chips, and a specific entry for one chip of the family. If we merge this patch, we either need a commit hook checking all possible chip variants for duplicate entries, or a builtin self test which checks for duplicates.

Even better would be not having to hardcode the flash chip name in scripts. What made you hardcode the chip name in the first place, and can we fix that?