Messages in this thread | | | Date | Sat, 10 May 2014 08:21:38 -0400 | From | Tejun Heo <> | Subject | Re: [PATCH 1/2] Add lib/glob.c |
| |
Hello,
On Fri, May 09, 2014 at 11:13:56PM -0400, George Spelvin wrote: > +config GLOB > + tristate > +# (Prompt disabled to reduce kbuild clutter until someone needs it.) > +# prompt "glob_match() function" > + help > + This option provides a glob_match function for performing simple > + text pattern matching. It is primarily used by the ATA code > + to blacklist particular drive models, but other device drivers > + may need similar functionality. > + > + All in-kernel drivers that require this function automatically > + select this option. Say N unless you are compiling an out-of > + tree driver which tells you it depend on it.
Just adding glob.o to lib-y should be enough. It will be excluded from linking if unused.
> +#ifdef UNITTEST > +/* To do a basic sanity test, "cc -DUNITTEST glob.c" and run a.out. */ > + > +#include <stdbool.h> > +#define __pure __attribute__((pure)) > +#define NOP(x) > +#define EXPORT_SYMBOL NOP /* Two stages to avoid checkpatch complaints */
These things tend to bitrot. Let's please keep testing harness out of tree.
> +#else > + > +#include <linux/module.h> > +#include <linux/glob.h> > + > +MODULE_DESCRIPTION("glob(7) matching"); > +MODULE_LICENSE("Dual MIT/GPL");
Do we make library routines separate modules usually?
... > +bool __pure > +glob_match(char const *pat, char const *str)
The whole thing fits in a single 80 column line, right?
bool __pure glob_match(char const *pat, char const *str)
> +{ > + /* > + * Backtrack to previous * on mismatch and retry starting one > + * character later in the string. Because * matches all characters > + * (no exception for /), it can be easily proved that there's > + * never a need to backtrack multiple levels. > + */ > + char const *back_pat = 0, *back_str = back_str;
Blank line here.
I haven't delved into the actual implementation. Looks sane on the first glance.
> +#ifdef UNITTEST > + > +/* Test code */ > +#include <stdio.h> > +#include <stdlib.h> > +struct glob_test { > + char const *pat, *str; > + bool expected; > +}; > + > +static void > +test(struct glob_test const *g) > +{ > + bool match = glob_match(g->pat, g->str); > + > + printf("\"%s\" vs. \"%s\": %s %s\n", g->pat, g->str, > + match ? "match" : "mismatch", > + match == g->expected ? "OK" : "*** ERROR ***"); > + if (match != g->expected) > + exit(1); > +} > + > +static struct glob_test const tests[] = { > + { "a", "a", true }, ... > + { "*ab*cd*", "abcabcabcabcefg", false } > +}; > + > +int > +main(void) > +{ > + size_t i; > + > + for (i = 0; i < sizeof(tests)/sizeof(*tests); i++) > + test(tests + i); > + > + return 0; > +} > + > +#endif /* UNITTEST */
Again, I don't really think the userland testing code belongs here. If you wanna keep them, please make it in-kernel selftesting. We don't really wanna keep code which can't get built and tested in kernel tree proper.
Thanks.
-- tejun
| |