From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753197AbbIPLXD (ORCPT <rfc822;w@1wt.eu>);
	Wed, 16 Sep 2015 07:23:03 -0400
Received: from mail-wi0-f178.google.com ([209.85.212.178]:36275 "EHLO
	mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752135AbbIPLV1 convert rfc822-to-8bit (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 16 Sep 2015 07:21:27 -0400
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        James Bottomley <JBottomley@Odin.com>, linux-kernel@vger.kernel.org,
        "K. Y. Srinivasan" <kys@microsoft.com>
Subject: Re: [PATCH v4 2/2] lib/test-string_helpers.c: add string_get_size() tests
Organization: D03
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>
X-Hashcash: 1:20:150916:andriy.shevchenko@linux.intel.com::MEnlTWmlwEOQk2vP:00000000000000000000000000000xD/
X-Hashcash: 1:20:150916:vkuznets@redhat.com::ZYjfvLFkdD7ja4mK:00000000000000000000000000000000000000000011Ny
X-Hashcash: 1:20:150916:linux-kernel@vger.kernel.org::Fcw2VYsuJy2dSlVD:0000000000000000000000000000000000/OA
X-Hashcash: 1:20:150916:akpm@linux-foundation.org::DgUxpiaq+F/sZ38m:0000000000000000000000000000000000002auT
X-Hashcash: 1:20:150916:jbottomley@odin.com::H7wRXAWJs4ypu9iH:0000000000000000000000000000000000000000003UVZ
X-Hashcash: 1:20:150916:kys@microsoft.com::BDw4rvQE8rzp1q9B:000000000000000000000000000000000000000000007xQZ
Date: Wed, 16 Sep 2015 13:21:24 +0200
In-Reply-To: <1442330454.8361.47.camel@linux.intel.com> (Andy Shevchenko's
	message of "Tue, 15 Sep 2015 18:20:54 +0300")
Message-ID: <87h9mu4nkb.fsf@rasmusvillemoes.dk>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.

[It would be nice if we could assert at compile-time that
strlen(exp_result) < sizeof(buf).]

> 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>.

>> +	pr_warn("expected: %s, got %s\n", exp_result, buf);
>
> Here I recommend to use single quotes (just to see empty strings)
>
> …expected '%s', got '%s'…

Good idea.