Hi Alexey, On 2026-09-18T12:13:49, Alexey Charkov <[email protected]> wrote: > test: lib: Check the GPT round trip against a real block device > > lib_test_gpt_blksz() drives gpt_fill_header() and gpt_fill_pte() with a > synthetic struct blk_desc. That is what lets it sweep block sizes no real > device here can provide, but it also means it cannot reach anything that > touches the medium, so the protective MBR, the header CRCs and every > conversion behind them go unchecked. > > Add a companion test that backs a blkmap device with plain memory, writes > a table with gpt_restore(), reads it back with gpt_verify_partitions() > and parses it with part_get_info(), then checks that the backup header > really did land in the last block. That covers set_protective_mbr(), > is_pmbr_valid(), gpt_verify_headers(), is_gpt_valid() and > part_get_info_efi(). > > The two tests are complementary rather than alternatives: gpt_blksz > covers the block size arithmetic across sizes a device cannot easily > offer, gpt_blkmap covers the read and write paths a synthetic descriptor > cannot reach. Neither replaces the sandbox tests in > test/py/tests/test_gpt.py, which exercise the gpt command against images > [...] > > test/lib/part_efi.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 144 insertions(+)
> diff --git a/test/lib/part_efi.c b/test/lib/part_efi.c > @@ -111,3 +114,144 @@ static int lib_test_gpt_blksz(struct unit_test_state > *uts) > + ut_assertok(blkmap_create(GPT_TEST_LABEL, &dev)); > + ut_assertok(blkmap_map_mem(dev, 0, lba, buf)); > + > + *bufp = buf; > + *devp = dev; If blkmap_map_mem() fails here, the blkmap is left bound and buf is leaked, since the caller never sees either of them. The same applies to every ut_assert in lib_test_gpt_blkmap() after the device is created. This matters more than the usual leak-on-failure because this test does not use UTF_DM, so the device stays under the real dm_root. After one failure, every later run of 'ut lib' fails in blkmap_create() with -EBUSY, which hides the original problem. Please can you move the body of the loop into a helper, so that the caller can do the blkmap_destroy() and free() regardless of the result? What do you think? > diff --git a/test/lib/part_efi.c b/test/lib/part_efi.c > @@ -111,3 +114,144 @@ static int lib_test_gpt_blksz(struct unit_test_state > *uts) > + /* two partitions, the second one running to the end */ > + memset(parts, '\0', sizeof(parts)); > + strcpy((char *)parts[0].name, "first"); > + strcpy((char *)parts[1].name, "second"); > + parts[0].size = 0x100; > + parts[1].size = 0x100; The comment does not match the code. Both partitions are 0x100 blocks, so on a 0x400-block disk the second one ends at 0x221, well short of last_usable_lba. Running to the end needs a size of 0, which gpt_verify_partitions() then skips in its size check. I suspect the fixed size is what you want, so please can you just fix the comment? > diff --git a/test/lib/part_efi.c b/test/lib/part_efi.c > @@ -111,3 +114,144 @@ static int lib_test_gpt_blksz(struct unit_test_state > *uts) > + ut_assertok(part_get_info(desc, 1, &info)); > + ut_asserteq_64(parts[0].size, info.size); > + ut_assertok(part_get_info(desc, 2, &info)); > + ut_asserteq_64(parts[1].size, info.size); Patch 2 fixes print_efiname(), but nothing here checks it. gpt_verify_partitions() goes through gpt_convert_efi_name_to_char() instead, so the name returned by part_get_info() is unchecked on big-endian. Please can you add a ut_asserteq_str() on info.name for each partition? Checking info.start against the expected first_usable_lba would be worthwhile too, since the start is not passed in and so gpt_verify_partitions() skips that check. Regards, Simon
