From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754043AbbI3GiN (ORCPT ); Wed, 30 Sep 2015 02:38:13 -0400 Received: from mga11.intel.com ([192.55.52.93]:5071 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753752AbbI3GiK (ORCPT ); Wed, 30 Sep 2015 02:38:10 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,611,1437462000"; d="scan'208";a="571065218" Message-ID: <1443595086.8361.272.camel@linux.intel.com> Subject: Re: [PATCH 4/4] test_printf: test printf family at runtime From: Andy Shevchenko To: Rasmus Villemoes Cc: Andrew Morton , Tejun Heo , linux-kernel@vger.kernel.org, Kees Cook Date: Wed, 30 Sep 2015 09:38:06 +0300 In-Reply-To: <874miefemv.fsf@rasmusvillemoes.dk> References: <1443202865-25533-1-git-send-email-linux@rasmusvillemoes.dk> <1443202865-25533-5-git-send-email-linux@rasmusvillemoes.dk> <1443431579.8361.234.camel@linux.intel.com> <874miefemv.fsf@rasmusvillemoes.dk> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.3-1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 "); > > > +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 Intel Finland Oy