Messages in this thread | | | Subject | Re: [PATCH v4 2/2] lib/test-string_helpers.c: add string_get_size() tests | From | Andy Shevchenko <> | Date | Wed, 16 Sep 2015 14:33:07 +0300 |
| |
On Wed, 2015-09-16 at 13:21 +0200, Rasmus Villemoes wrote: > On Tue, Sep 15 2015, Andy Shevchenko < > andriy.shevchenko@linux.intel.com> wrote: > > > On Tue, 2015-09-15 at 15:55 +0200, Vitaly Kuznetsov wrote: > > > +static __init void test_string_get_size_one(u64 size, u64 > > > blk_size, > > > + const enum > > > string_size_units units, > > > + const char > > > *exp_result) > > > +{ > > > + char buf[16]; > > > + > > > + string_get_size(size, blk_size, units, buf, > > > sizeof(buf)); > > > + if (!memcmp(buf, exp_result, strnlen(exp_result, > > > sizeof(buf) > > > - 1) + 1)) > > > > Actually you don't need to do this +- 1. Either you will have '\0' > > or > > not, it will be checked by memcmp() anyway. > > > > Thus, > > memcmp(buf, exp_result, strnlen(exp_result, sizeof(buf))). > > Huh? How does that ensure that string_get_size put a '\0' at the > right > spot? We do need the comparison to also cover the terminating '\0' in > exp_result.
Ah, you are right.
But seems we have length of the exp_result always smaller than buffer size, so, would we change this to memcmp(…, strlen(exp_result) + 1); ?
> [It would be nice if we could assert at compile-time that > strlen(exp_result) < sizeof(buf).]
Interesting if BUILD_BUG_ON can help here. Can we use sizeof(exp_result) since all of them are literal constants?
> > > Perhaps one line comment here > > /* Make sure that buf will be always NULL-terminated */ > > > > > + buf[sizeof(buf) - 1] = '\0'; > > <bikeshed>Could we pretty-please use different names for 0 the > pointer > and 0 the character, say in this case nul or NUL or '\0' or simply > 0. Also, I don't see the value of the comment; that line is a totally > standard idiom.</bikeshed>.
Got your point.
-- Andy Shevchenko <andriy.shevchenko@linux.intel.com> Intel Finland Oy
| |