On 22.04.22 15:33, Rebecca Mckeever wrote:
> Update comments in memblock_remove_*() functions to match the style used
> in tests/alloc_*.c by rewording to make the expected outcome more apparent
> and, if more than one memblock is involved, adding a visual of the
> memory blocks.
>
> If the comment has an extra column of spaces, remove the extra space at
> the beginning of each line for consistency and to conform to Linux kernel
> coding style.
>
> Signed-off-by: Rebecca Mckeever <remckee0@xxxxxxxxx>
> ---
> tools/testing/memblock/tests/basic_api.c | 101 +++++++++++++++++------
> 1 file changed, 75 insertions(+), 26 deletions(-)
>
> diff --git a/tools/testing/memblock/tests/basic_api.c b/tools/testing/memblock/tests/basic_api.c
> index 75cd7479ee54..08847dc5065e 100644
> --- a/tools/testing/memblock/tests/basic_api.c
> +++ b/tools/testing/memblock/tests/basic_api.c
> @@ -550,14 +550,21 @@ static int memblock_reserve_checks(void)
> return 0;
> }
>
> - /*
> - * A simple test that tries to remove the first entry of the array of
> - * available memory regions. By "removing" a region we mean overwriting it
> - * with the next region in memblock.memory. To check this is the case, the
> - * test adds two memory blocks and verifies that the value of the latter
> - * was used to erase r1 region. It also checks if the region counter and
> - * total size were updated to expected values.
> - */
> +/*
> + * A simple test that tries to remove the first entry of the array of
> + * with the next region in memblock.memory:
I failed to parse this sentence "of the array of with".
> + *
> + * | ...... +----------------+ |
> + * | : r1 : | r2 | |
> + * +--+----+----------+----------------+--+
> + * ^
> + * |
> + * rgn.base
> + *
> + * Expect to add two memory blocks r1 and r2 and then remove r1 region
> + * so that r2 is the first available region. The region counter and
> + * total size are updated.
> + */
> static int memblock_remove_simple_check(void)
> {
> struct memblock_region *rgn;
> @@ -587,11 +594,22 @@ static int memblock_remove_simple_check(void)
> return 0;
> }
>
> - /*
> - * A test that tries to remove a region that was not registered as available
> - * memory (i.e. has no corresponding entry in memblock.memory). It verifies
> - * that array, regions counter and total size were not modified.
> - */
> +/*
> + * A test that tries to remove a region that was not registered as available
> + * memory (i.e. has no corresponding entry in memblock.memory):
> + *
> + * +----------------+
> + * | r2 |
> + * +----------------+
> + * | +----+ |
> + * | | r1 | |
> + * +--+----+------------------------------+
> + * ^
> + * |
> + * rgn.base
> + *
> + * Expect the array, regions counter and total size to not be modified.
> + */
> static int memblock_remove_absent_check(void)
> {
> struct memblock_region *rgn;
> @@ -622,10 +640,21 @@ static int memblock_remove_absent_check(void)
>
> /*
> * A test that tries to remove a region which overlaps with the beginning of
Wonder if it makes sense to mention r2 here. ("a region r2 ...") -- to
be precise, to always reference r1 and r2 accordingly instead of e.g.,
"the first entry". Same applies to the other function documentation you
touch in this patch.
--
Thanks,
David / dhildenb
