From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752424AbbIPLfV (ORCPT ); Wed, 16 Sep 2015 07:35:21 -0400 Received: from mga11.intel.com ([192.55.52.93]:27597 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750989AbbIPLfT (ORCPT ); Wed, 16 Sep 2015 07:35:19 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,538,1437462000"; d="scan'208";a="805616963" Message-ID: <1442403187.8361.62.camel@linux.intel.com> Subject: Re: [PATCH v4 2/2] lib/test-string_helpers.c: add string_get_size() tests From: Andy Shevchenko To: Rasmus Villemoes Cc: Vitaly Kuznetsov , Andrew Morton , James Bottomley , linux-kernel@vger.kernel.org, "K. Y. Srinivasan" Date: Wed, 16 Sep 2015 14:33:07 +0300 In-Reply-To: <87h9mu4nkb.fsf@rasmusvillemoes.dk> References: <1442325322-23366-1-git-send-email-vkuznets@redhat.com> <1442325322-23366-3-git-send-email-vkuznets@redhat.com> <1442330454.8361.47.camel@linux.intel.com> <87h9mu4nkb.fsf@rasmusvillemoes.dk> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.16.3-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 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'; > > 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.. Got your point. -- Andy Shevchenko Intel Finland Oy