Messages in this thread | | | Subject | Re: [PATCH v1 3/7] test_hexdump: go through all possible lengths of buffer | From | Andy Shevchenko <> | Date | Fri, 20 Nov 2015 18:58:04 +0200 |
| |
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
| |