From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1163224AbbKTQ57 (ORCPT ); Fri, 20 Nov 2015 11:57:59 -0500 Received: from mga14.intel.com ([192.55.52.115]:26187 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161981AbbKTQ56 (ORCPT ); Fri, 20 Nov 2015 11:57:58 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,323,1444719600"; d="scan'208";a="855854510" Message-ID: <1448038684.31665.179.camel@linux.intel.com> Subject: Re: [PATCH v1 3/7] test_hexdump: go through all possible lengths of buffer From: Andy Shevchenko To: Rasmus Villemoes Cc: Andrew Morton , linux-kernel@vger.kernel.org Date: Fri, 20 Nov 2015 18:58:04 +0200 In-Reply-To: <87twois3or.fsf@rasmusvillemoes.dk> References: <1447259718-19647-1-git-send-email-andriy.shevchenko@linux.intel.com> <1447259718-19647-4-git-send-email-andriy.shevchenko@linux.intel.com> <87twois3or.fsf@rasmusvillemoes.dk> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2015-11-19 at 11:07 +0100, Rasmus Villemoes wrote: > On Wed, Nov 11 2015, Andy Shevchenko om> wrote: > > > When test for overflow do iterate the buffer length in a range > > 0 .. BUF_SIZE. > > > > Signed-off-by: Andy Shevchenko > > --- > >  lib/test_hexdump.c | 20 ++++++++++---------- > >  1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c > > index ed7c6a7..15a6440 100644 > > --- a/lib/test_hexdump.c > > +++ b/lib/test_hexdump.c > > @@ -126,17 +126,17 @@ static void __init test_hexdump_set(int > > rowsize, bool ascii) > >   test_hexdump(len, rowsize, 1, ascii); > >  } > >   > > -static void __init test_hexdump_overflow(bool ascii) > > +static void __init test_hexdump_overflow(size_t buflen, bool > > ascii) > >  { > > - char buf[56]; > > + char buf[TEST_HEXDUMP_BUF_SIZE]; > >   const char *t = test_data_1_le[0]; > > - size_t l = get_random_int() % sizeof(buf); > > + size_t l = buflen; > >   bool a; > >   int e, r; > >   > >   memset(buf, ' ', sizeof(buf)); > >   > > - r = hex_dump_to_buffer(data_b, 1, 16, 1, buf, l, ascii); > > + r = hex_dump_to_buffer(data_b, 1, 16, 1, buf, buflen, > > ascii); > >   > >   if (ascii) > >   e = 50; > > @@ -144,7 +144,7 @@ static void __init test_hexdump_overflow(bool > > ascii) > >   e = 2; > >   buf[e + 2] = '\0'; > >   > > - if (!l) { > > + if (!buflen) { > >   a = r == e && buf[0] == ' '; > >   } else if (l < 3) { > >   a = r == e && buf[0] == '\0'; > > > Why keep the variable l when it is just a synonym for the new > parameter buflen? It is quite confusing that you change some but not > all occurrences of l to buflen. If you want to make the diff minimal > but still have a descriptive parameter name, just keep the 'size_t l > = > buflen;' assignment and don't otherwise refer to buflen. But I think > it's better to eliminate 'l' and just change everything to > buflen. Don't mix the two approaches, though. Okay, I got it for the future, though the series is already in linux- next, so do we really need to re-hack half of it because of that? I suppose everything else you noticed I may send as one follow up patch. > > Rasmus -- Andy Shevchenko Intel Finland Oy