Re: [PATCH v3] dynamic_debug: add wildcard support to filter files/functions/modules

[Changbin Du]

2013/10/30 Joe Perches <joe@xxxxxxxxxxx>:
> On Tue, 2013-10-29 at 21:33 +0800, Du, Changbin wrote:
>> This patch add wildcard '*'(matches zero or more characters) and '?'
>> (matches one character) support when qurying debug flags.
>
> Hi again.  Some trivial notes and a possible logic error:
>

>> +/* check if the string matches given pattern which includes wildcards */
>> +static bool match_pattern(const char *pattern, const char *string)
>> +{
>> +     const char *s = string,
>> +                *p = pattern;
>
> This sort of alignment is pretty unusual.
> Most kernel uses just repeat the type like:
>
>         const char *s = string;
>         const char *p = pattern;
>
I think so.

>> +     bool star = 0;
>
>         bool star = false;
>
>> +
>> +     while (*s) {
>> +             switch (*p) {
>> +             case '?':
>> +                     ++s, ++p;
>> +                     break;
>> +             case '*':
>> +                     star = true;
>> +                     string = s;
>> +                     if (!*++p)
>> +                             return true;
>> +                     pattern = p;;
>
> repeated ;
> Running your patches through checkpatch should find
> this sort of defect.
>

Sorry, it's may fault. I forgot to check it using the tool.

>> +                     break;
>> +             default:
>> +                     if (*s != *p) {
>> +                             if (!star)
>> +                                     return false;
>> +                             string++;
>> +                             s = string;
>> +                             p = pattern;
>> +                             break;
>> +                     }
>> +                     ++s, ++p;
>
> Maybe nicer with an if/else, I think you're still
> missing a reset of "star = false;" and I also think
> it's better to use a break here too.
>
>                 if (*s == *p) {
>                         s++;
>                         p++;
>                         star = false;
>                 } else {
>                         if (!star)
>                                 return false;
>                         string++;
>                         s = string;
>                         p = pattern;
>                 }
>                 break;

I have run loss of test before sending patch. all case passed. But I
will double check if need reset star flag. really thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/