On Thu, Jun 19, 2025 at 05:04:23PM -0500, Sam Protsenko wrote:
> On Fri, Feb 7, 2025 at 7:26 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Sam,
> >
> > On Fri, 7 Feb 2025 at 18:11, Sam Protsenko <semen.protsenko at linaro.org> wrote:
> > >
> > > On Tue, Jan 14, 2025 at 7:14 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > On Sat, 11 Jan 2025 at 20:42, Sam Protsenko <semen.protsenko at linaro.org> 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 <semen.protsenko at linaro.org>
> > > > > ---
> > > > > boot/bootmeth-uclass.c | 16 ++++++++++++----
> > > > > 1 file changed, 12 insertions(+), 4 deletions(-)
> > > > >
> > > >
> > > > Reviewed-by: Simon Glass <sjg at chromium.org>
> > > >
> > > > 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250619/142ed2b7/attachment.sig>