Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1274266

Re: [PATCH v1 3/7] test_hexdump: go through all possible lengths of buffer

Show all headers | View raw

On Thu, 2015-11-19 at 11:07 +0100, Rasmus Villemoes wrote:
> On Wed, Nov 11 2015, Andy Shevchenko <andriy.shevchenko@linux.intel.c
> om> wrote:
> 
> > When test for overflow do iterate the buffer length in a range
> > 0 .. BUF_SIZE.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  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 <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH v1 3/7] test_hexdump: go through all possible lengths of buffer Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-11-19 11:10 +0100
  Re: [PATCH v1 3/7] test_hexdump: go through all possible lengths of  buffer Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-11-20 18:00 +0100
    Re: [PATCH v1 3/7] test_hexdump: go through all possible lengths of buffer Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2015-11-23 10:00 +0100
      Re: [PATCH v1 3/7] test_hexdump: go through all possible lengths of buffer Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-26 16:30 +0100

csiph-web