On Thu, Jun 19, 2025 at 05:04:23PM -0500, Sam Protsenko wrote: > On Fri, Feb 7, 2025 at 7:26 PM Simon Glass <[email protected]> wrote: > > > > Hi Sam, > > > > On Fri, 7 Feb 2025 at 18:11, Sam Protsenko <[email protected]> > > wrote: > > > > > > On Tue, Jan 14, 2025 at 7:14 AM Simon Glass <[email protected]> wrote: > > > > > > > > On Sat, 11 Jan 2025 at 20:42, Sam Protsenko > > > > <[email protected]> wrote: > > > > > > > > > > Free memory allocated for 'order' (array of bootmeths) on error paths > > > > > in > > > > > bootmeth_setup_iter_order() function. > > > > > > > > > > Fixes: c627cfc14c08 ("bootstd: Allow scanning for global bootmeths > > > > > separately") > > > > > Fixes: 10d16faa436c ("bootstd: Detect empty bootmeth") > > > > > Signed-off-by: Sam Protsenko <[email protected]> > > > > > --- > > > > > boot/bootmeth-uclass.c | 16 ++++++++++++---- > > > > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > > > > > > > > > > Reviewed-by: Simon Glass <[email protected]> > > > > > > > > How about adding a test to check there is no leak? You can see > > > > test/lib/abuf.c for similar tests. > > > > > > That might be a good idea, and I already have the test code handy. But > > > frankly I wasn't able to successfully run the current bootstd test in > > > sandbox, it fails before it can get to memleak check I added. Firstly, > > > when I do: > > > > > > $ make sandbox_defconfig > > > $ make -j32 > > > $ ./u-boot -T -c "ut bootstd" > > > > > > it says: > > > > > > MMC: Can't map file 'mmc1.img': Invalid argument > > > > > > and I can see only empty (zero size) mmc1.img in my U-Boot source code > > > root dir. Then I was able to trigger its creation (I guess) by running > > > Python suite somehow, so it generated 20 MiB mmc1.img. But even with > > > that added, I hit this error: > > > > > > test/boot/bootdev.c:218, bootdev_test_order(): > > > 0 == bootflow_scan_first(((void *)0), ((void *)0), &iter, 0, > > > &bflow): > > > Expected 0x0 (0), got 0xffffffed (-19) > > > > > > I've read corresponding doc [1] ("Tests" section), but still don't > > > understand how can I successfully run the bootstd test locally. If you > > > can help me out, I can send the memleak test you mentioned in v2. > > > > You can run the bootstd tests with something like: > > > > test/py/test.py -B sandbox --build-dir /tmp/b/sandbox -k bootstd > > > > although it sounds like you have already figured that out. After you > > do it once (to create the images) you can run the C tests directly as > > you did above, but yes, sadly, some of the tests are affected by > > others (with bootstd). You could take a look if you like. > > > > Is it possible to just apply this patch without a test? It's clearly > fixing a memleak. Not that I'm lazy or anything, I came up with a test > like below, but I don't want to send something that is quite hard to > test. If you think this test is really essential, I can send it once > bootstd tests are fixed and can be run in one command without any > extra non-documented work. > > 8<---------------------------------------------------------------------->8 > diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c > index 9af947868707..f4746ee4fb61 100644 > --- a/test/boot/bootdev.c > +++ b/test/boot/bootdev.c > @@ -194,6 +194,7 @@ static int bootdev_test_order(struct unit_test_state *uts) > { > struct bootflow_iter iter; > struct bootflow bflow; > + ulong start; > > test_set_skip_delays(true); > > @@ -201,6 +202,8 @@ static int bootdev_test_order(struct unit_test_state *uts) > bootstd_reset_usb(); > ut_assertok(run_command("usb start", 0)); > > + start = ut_check_free(); > + > /* > * First try the order set by the bootdev-order property > * Like all sandbox unit tests this relies on the devicetree setting > up > @@ -277,6 +280,9 @@ static int bootdev_test_order(struct unit_test_state *uts) > iter.dev_used[3]->name); > bootflow_iter_uninit(&iter); > > + /* Check for memory leaks */ > + ut_assertok(ut_check_delta(start)); > + > return 0; > } > BOOTSTD_TEST(bootdev_test_order, UTF_DM | UTF_SCAN_FDT); > 8<---------------------------------------------------------------------->8
Yes, we can do this in a follow-up I think. I'll take this series to -next soon. -- Tom
signature.asc
Description: PGP signature
