| Message ID | 4C2CB497.3000701@teksavvy.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
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
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
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
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
--- 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++;
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