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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
