[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ef1620a2-dbc6-911e-5cd0-e860ad7772e0@redhat.com>
Date: Thu, 30 Jun 2022 12:17:48 +0200
From: David Hildenbrand <david@...hat.com>
To: Rebecca Mckeever <remckee0@...il.com>
Cc: Mike Rapoport <rppt@...nel.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/4] memblock tests: add verbose output to memblock
tests
On 29.06.22 19:07, Rebecca Mckeever wrote:
> On Wed, Jun 29, 2022 at 01:34:54PM +0200, David Hildenbrand wrote:
>> On 28.06.22 00:34, Rebecca Mckeever wrote:
>>> Add and use functions and macros for printing verbose testing output.
>>>
>>> If the Memblock simulator was compiled with VERBOSE=1:
>>> - prefix_push(): appends the given string to a prefix string that will be
>>> printed in test_fail() and test_pass*().
>>>
>>> - prefix_pop(): removes the last prefix from the prefix string.
>>>
>>> - prefix_reset(): clears the prefix string.
>>>
>>> - test_fail(): prints a message after a test fails containing the test
>>> number of the failing test and the prefix.
>>>
>>> - test_pass(): prints a message after a test passes containing its test
>>> number and the prefix.
>>>
>>> - test_print(): prints the given formatted output string.
>>>
>>> - test_pass_pop(): runs test_pass() followed by prefix_pop().
>>>
>>> - PREFIX_PUSH(): runs prefix_push(__func__).
>>>
>>> If the Memblock simulator was not compiled with VERBOSE=1, these
>>> functions/macros do nothing.
>>>
>>> Add the assert wrapper macros ASSERT_EQ(), ASSERT_NE(), and ASSERT_LT().
>>> If the assert condition fails, these macros call test_fail() before
>>> executing assert().
>>>
>>> Signed-off-by: Rebecca Mckeever <remckee0@...il.com>
>>
>>
>> [...]
>>
>>>
>>> diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
>>> index 62d3191f7c9a..e55b2a8bf0ff 100644
>>> --- a/tools/testing/memblock/tests/common.c
>>> +++ b/tools/testing/memblock/tests/common.c
>>> @@ -4,8 +4,12 @@
>>>
>>> #define INIT_MEMBLOCK_REGIONS 128
>>> #define INIT_MEMBLOCK_RESERVED_REGIONS INIT_MEMBLOCK_REGIONS
>>> +#define PREFIXES_LEN_MAX 256
>>> +#define DELIM ": "
>>> +#define DELIM_LEN strlen(DELIM)
>>
>> Why not simply
>>
>> #define PREFIXES_MAX 15
>> static const char * __maybe_unused prefixes[PREFIXES_MAX];
>> static int nr_prefixes;
>>
>> And then simply insert/clear the corresponding prefixes[] pointer and
>> update nr_prefixes?
>>
>> When printing, you only have to walk prefixes from 0 ... nr_prefixes - 1
>> and print the values.
>>
>> Avoids any string modifications.
>>
> What is nr_prefixes? Number of prefixes? Currently, the longest prefix is
> 49 characters (alloc_try_nid_bottom_up_reserved_with_space_check), so I
> think PREFIXES_MAX would need to be at least 52 (including the delimiter),
> but let me know if I'm misunderstanding.
nr_prefixes would be the current number of prefixes (not the length).
You be storing pointers to strings in the constant pool, not copying the
strings over.
--
Thanks,
David / dhildenb
Powered by blists - more mailing lists