Messages in this thread | | | From | Rasmus Villemoes <> | Subject | Re: [PATCH v1 3/7] test_hexdump: go through all possible lengths of buffer | Date | Thu, 19 Nov 2015 11:07:16 +0100 |
| |
On Wed, Nov 11 2015, Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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.
Rasmus
| |