Messages in this thread | | | Subject | Re: [PATCH 4/4] test_printf: test printf family at runtime | From | Andy Shevchenko <> | Date | Wed, 30 Sep 2015 09:38:06 +0300 |
| |
On Mon, 2015-09-28 at 22:55 +0200, Rasmus Villemoes wrote: > 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.
> > > +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.
It's still better to distinguish what function does by names test vs. __test confuses me.
But whatever, this is a test suite, not an actual code anyway.
> >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.
So, what was the reason to add them in the first place?
> > > + > > > +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.
You are the author, your choice. I'm okay with any applicable type.
-- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy
| |