Patchwork [1/3] libata: glob_match for ata_device_blacklist

login
register
mail settings
Submitter Mark Lord
Date July 1, 2010, 3:30 p.m.
Message ID <4C2CB497.3000701@teksavvy.com>
Download mbox | patch
Permalink /patch/57556/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Mark Lord - July 1, 2010, 3:30 p.m.
Replace rudimentry pattern matching with more capable shell-style globbing.
This will enable shrinking ata_device_blacklist[] table in subsequent patches,
and helps with future editions to the table, such as matching only the end
of a firmware revision string etc..

Signed-off-by: Mark Lord <mlord@pobox.com>
---

Eventually, this function should move out of libata into lib/string.c or similar (?).
But since following patches in this series require it for libata,
let's start with it there, and move things later on.

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Lord - July 1, 2010, 3:34 p.m.
On 01/07/10 11:30 AM, Mark Lord wrote:
>
> Replace rudimentry pattern matching with more capable shell-style globbing.
> This will enable shrinking ata_device_blacklist[] table in subsequent patches,
..

I am working on this, because a future device to be added (not ready yet)
to the blacklist table requires matching on only the end of the firmware string.
The old pattern matcher cannot handle that case, so..

In the meanwhile, this gives us decent memory savings,
and makes the list easier to maintain even for current entries.

Cheers

Mark
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
James Bottomley - July 1, 2010, 3:36 p.m.
On Thu, 2010-07-01 at 11:30 -0400, Mark Lord wrote:
> Replace rudimentry pattern matching with more capable shell-style globbing.
> This will enable shrinking ata_device_blacklist[] table in subsequent patches,
> and helps with future editions to the table, such as matching only the end
> of a firmware revision string etc..

So we have other identical cases of this list to flag matching ... like
in SCSI.  How about we move the mechanics to lib and share the code?

James


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Lord - July 1, 2010, 3:44 p.m.
On 01/07/10 11:36 AM, James Bottomley wrote:
> On Thu, 2010-07-01 at 11:30 -0400, Mark Lord wrote:
>> Replace rudimentry pattern matching with more capable shell-style globbing.
>> This will enable shrinking ata_device_blacklist[] table in subsequent patches,
>> and helps with future editions to the table, such as matching only the end
>> of a firmware revision string etc..
>
> So we have other identical cases of this list to flag matching ... like
> in SCSI.  How about we move the mechanics to lib and share the code?


I agree wholeheartedly.  But first it has to get upstream.
And it has follow-on patches that depend on it,
so initially I'd like to push it up via the libata tree.
And then.. as in the original post:

> Eventually, this function should move out of libata into lib/string.c or similar (?).
> But since following patches in this series require it for libata,
> let's start with it there, and move things later on.


Cheers!

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Garzik - July 1, 2010, 7:24 p.m.
On 07/01/2010 11:30 AM, Mark Lord wrote:
>
> Replace rudimentry pattern matching with more capable shell-style globbing.
> This will enable shrinking ata_device_blacklist[] table in subsequent
> patches,
> and helps with future editions to the table, such as matching only the end
> of a firmware revision string etc..
>
> Signed-off-by: Mark Lord <mlord@pobox.com>
> ---
>
> Eventually, this function should move out of libata into lib/string.c or
> similar (?).
> But since following patches in this series require it for libata,
> let's start with it there, and move things later on.

hmmm, can you resend this series as an attachment?  The patch is failing 
miserably here, both with 'git am' and patch(1).  I don't see anything 
obviously wrong with the patch other than line numbers -- did you diff 
against the latest upstream kernel (torvalds/linux-2.6.git)?

Adding "-l" to patch(1) causes hunk #2 of patch #1 to succeed, but being 
the smaller hunk, that's the less useful of the two.

	Jeff




[jgarzik@bd libata-dev]$ patch -l --verbose drivers/ata/libata-core.c < 
.git/rebase-apply/patch
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|---
|
|Eventually, this function should move out of libata into lib/string.c 
or similar (?).
|But since following patches in this series require it for libata,
|let's start with it there, and move things later on.
|
|--- 2.6.34/drivers/ata/libata-core.c	2010-05-16 17:17:36.000000000 -0400
|+++ linux/drivers/ata/libata-core.c	2010-07-01 11:08:10.509159666 -0400
--------------------------
Patching file drivers/ata/libata-core.c using Plan A...
Hunk #1 FAILED at 4413.
Hunk #2 succeeded at 4359 (offset -87 lines).
1 out of 2 hunks FAILED -- saving rejects to file 
drivers/ata/libata-core.c.rej
Hmm...  Ignoring the trailing garbage.
done

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

--- 2.6.34/drivers/ata/libata-core.c	2010-05-16 17:17:36.000000000 -0400
+++ linux/drivers/ata/libata-core.c	2010-07-01 11:08:10.509159666 -0400
@@ -4413,29 +4413,65 @@ 
  	{ }
  };
  
-static int strn_pattern_cmp(const char *patt, const char *name, int wildchar)
+/**
+ *	glob_match - match a text string against a glob-style pattern
+ *	@text: the string to be examined
+ *	@pattern: the glob-style pattern to be matched against
+ *
+ *	Either/both of text and pattern can be empty strings.
+ *
+ *	Match text against a glob-style pattern, with wildcards and simple sets:
+ *
+ *		?	matches any single character.
+ *		*	matches any run of characters.
+ *		[xyz]	matches a single character from the set: x, y, or z.
+ *
+ *	Note: hyphenated ranges [0-9] are _not_ supported here.
+ *	The special characters ?, [, or *, can be matched using a set, eg. [*]
+ *
+ *	Example patterns:  "SD1?",  "SD1[012345]",  "*R0",  SD*1?[012]*xx"
+ *
+ *	This function uses one level of recursion per '*' in pattern.
+ *	Since it calls _nothing_ else, and has _no_ explicit local variables,
+ *	this will not cause stack problems for any reasonable use here.
+ *
+ *	RETURNS:
+ *	0 on match, 1 otherwise.
+ */
+static int glob_match (const char *text, const char *pattern)
  {
-	const char *p;
-	int len;
+	do {
+		/* Match single character or a '?' wildcard */
+		if (*text == *pattern || *pattern == '?') {
+			if (!*pattern++)
+				return 0;  /* End of both strings: match */
+		} else {
+			/* Match single char against a '[' bracketed ']' pattern set */
+			if (!*text || *pattern != '[')
+				break;  /* Not a pattern set */
+			while (*++pattern && *pattern != ']' && *text != *pattern);
+			if (!*pattern || *pattern == ']')
+				return 1;  /* No match */
+			while (*pattern && *pattern++ != ']');
+		}
+	} while (*++text && *pattern);
  
-	/*
-	 * check for trailing wildcard: *\0
-	 */
-	p = strchr(patt, wildchar);
-	if (p && ((*(p + 1)) == 0))
-		len = p - patt;
-	else {
-		len = strlen(name);
-		if (!len) {
-			if (!*patt)
-				return 0;
-			return -1;
+	/* Match any run of chars against a '*' wildcard */
+	if (*pattern == '*') {
+		if (!*++pattern)
+			return 0;  /* Match: avoid recursion at end of pattern */
+		/* Loop to handle additional pattern chars after the wildcard */
+		while (*text) {
+			if (glob_match(text, pattern) == 0)
+				return 0;  /* Remainder matched */
+			++text;  /* Absorb (match) this char and try again */
  		}
  	}
-
-	return strncmp(patt, name, len);
+	if (!*text && !*pattern)
+		return 0;  /* End of both strings: match */
+	return 1;  /* No match */
  }
-
+
  static unsigned long ata_dev_blacklisted(const struct ata_device *dev)
  {
  	unsigned char model_num[ATA_ID_PROD_LEN + 1];
@@ -4446,10 +4482,10 @@ 
  	ata_id_c_string(dev->id, model_rev, ATA_ID_FW_REV, sizeof(model_rev));
  
  	while (ad->model_num) {
-		if (!strn_pattern_cmp(ad->model_num, model_num, '*')) {
+		if (!glob_match(model_num, ad->model_num)) {
  			if (ad->model_rev == NULL)
  				return ad->horkage;
-			if (!strn_pattern_cmp(ad->model_rev, model_rev, '*'))
+			if (!glob_match(model_rev, ad->model_rev))
  				return ad->horkage;
  		}
  		ad++;