Re: [PATCH v4 2/4] memblock tests: add verbose output to memblock tests

[David Hildenbrand <david@redhat.com>]

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