From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752651AbbINWAi (ORCPT ); Mon, 14 Sep 2015 18:00:38 -0400 Received: from mail-wi0-f182.google.com ([209.85.212.182]:35340 "EHLO mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752059AbbINWAO (ORCPT ); Mon, 14 Sep 2015 18:00:14 -0400 From: Rasmus Villemoes To: Vitaly Kuznetsov Cc: Andrew Morton , Andy Shevchenko , James Bottomley , linux-kernel@vger.kernel.org, "K. Y. Srinivasan" Subject: Re: [PATCH v3 2/2] lib/test-string_helpers.c: add string_get_size() tests Organization: D03 References: <1442249150-31650-1-git-send-email-vkuznets@redhat.com> <1442249150-31650-3-git-send-email-vkuznets@redhat.com> X-Hashcash: 1:20:150914:jbottomley@odin.com::ThBRET0A+WzBeFdM:0000000000000000000000000000000000000000001Hl1 X-Hashcash: 1:20:150914:andriy.shevchenko@linux.intel.com::5Phuoqlwob9BaPui:00000000000000000000000000001xTe X-Hashcash: 1:20:150914:kys@microsoft.com::vrfcWidr/tsFsuN1:000000000000000000000000000000000000000000003mSp X-Hashcash: 1:20:150914:akpm@linux-foundation.org::wC9VTLq+2o+wPING:00000000000000000000000000000000000048rP X-Hashcash: 1:20:150914:linux-kernel@vger.kernel.org::JBqzH8RHPTH5gR6E:0000000000000000000000000000000008OdH X-Hashcash: 1:20:150914:vkuznets@redhat.com::iZ0FVb2UlUhfWh1t:000000000000000000000000000000000000000000A/25 Date: Tue, 15 Sep 2015 00:00:11 +0200 In-Reply-To: <1442249150-31650-3-git-send-email-vkuznets@redhat.com> (Vitaly Kuznetsov's message of "Mon, 14 Sep 2015 18:45:50 +0200") Message-ID: <87y4g8wtjo.fsf@rasmusvillemoes.dk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Sep 14 2015, 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[256]; > + > + string_get_size(size, blk_size, units, buf, sizeof(buf)); > + if (!strncmp(buf, exp_result, min(sizeof(buf), strlen(exp_result)))) > + return; Nits: It probably makes sense to also test that string_get_size '\0'-terminates the buffer, so I'd spell this if (!memcmp(buf, exp_result, min(sizeof(buf), strlen(exp_result)+1))) With a generous stack buffer, that min() will always evaluate to the strlen(exp_result)+1. On that note: Maybe 256 is a bit excessive. I don't think this will run very deep in the kernel stack, but the code might get copy-pasted somewhere else. 16 should be plenty. > + pr_warn("Test 'test_string_get_size_one' failed!\n"); > + pr_warn("string_get_size(size = %llu, blk_size = %llu, units = %d\n", > + size, blk_size, units); [There's probably no pretty way of getting from units to a text representation, but it's slightly annoying to have to check the source for the enum definition to figure out what units=0 or units=1 means.] > + pr_warn("expected: %s, got %s\n", exp_result, buf); In case we failed to '\0'-terminate buf, we might want to print it with "%.*s", (int)sizeof(buf), buf. But maybe I'm just overly paranoid. Rasmus