Messages in this thread | | | From | Rasmus Villemoes <> | Subject | Re: [PATCH 4/4] test_printf: test printf family at runtime | Date | Mon, 28 Sep 2015 22:55:04 +0200 |
| |
On Mon, Sep 28 2015, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> On Fri, 2015-09-25 at 19:41 +0200, Rasmus Villemoes wrote: >> This adds a simple module for testing the kernel's printf >> facilities. Previously, some %p extensions have caused a wrong return >> value in case the entire output didn't fit and/or been unusable in >> kasprintf(). This should help catch such issues. Also, it should help >> ensure that changes to the formatting algorithms don't break >> anything. >> >> I'm not sure if we have a struct dentry or struct file lying around >> at >> boot time or if we can fake one, but most %p extensions should be >> testable, as should the ordinary number and string formatting. >> >> The nature of vararg functions means we can't use a more conventional >> table-driven approach. >> >> For now, this is mostly a skeleton; contributions are very >> welcome. Some tests are/will be slightly annoying to write, since the >> expected output depends on stuff like CONFIG_*, sizeof(long), runtime >> values etc. > > Few comments below. > >> + >> +#define test(expect, fmt, ...) \ >> + __test(expect, strlen(expect), fmt, ##__VA_ARGS__) > > Would be __test_m[em] / __test_s[tr] to distinguish them by name?
Erh, no. The 'mem' version will only be used in a very few cases, and I really want the simple name "test" for the common case.
> And might be inline function?
That'd make the vararg handling more cumbersome.
>> +static void __init >> +test_basic(void) >> +{ >> + test("", ""); >> + test("100%", "100%%"); >> + test("xxx%yyy", "xxx%cyyy", '%'); >> + __test("xxx\0yyy", 7, "xxx%cyyy", '\0'); > > And such pieces will be look better > > __test_str("xxx%yyy", "xxx%cyyy", '%'); > __test_mem("xxx\0yyy", 7, "xxx%cyyy", '\0');
I don't agree.
>> + >> +static void __init >> +netdev_features(void) >> +{ >> +} >> + >> + > > Maybe commentary delimiter here and above where you have double empty > line.
And say what? I can avoid double empty lines if they bother you.
>> + >> + return 0; > > Do we need this module in a memory?
I guess not. At first I thought it didn't really matter since all functions and data are __init, but I suppose a little metadata would stick around if loading is "successful". Will fix.
>> + >> +MODULE_AUTHOR("Rasmus Villemoes <linux@rasmusvillemoes.dk>"); >> +MODULE_LICENSE("GPL"); > > GPL or ?..
Honestly, I don't really care. Would you like BSD/GPL or what? I just copied from the majority of MODULE_LICENSE() instances.
Rasmus
| |