Hi Tom, On Fri, 13 Feb 2026 at 13:29, Tom Rini <[email protected]> wrote: > > On Fri, Feb 13, 2026 at 01:19:54PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Fri, 13 Feb 2026 at 11:01, Tom Rini <[email protected]> wrote: > > > > > > On Fri, Feb 13, 2026 at 10:53:25AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Fri, 13 Feb 2026 at 09:57, Tom Rini <[email protected]> wrote: > > > > > > > > > > On Tue, Dec 09, 2025 at 08:17:06AM -0600, Tom Rini wrote: > > > > > > On Tue, Dec 09, 2025 at 12:16:33PM +0200, Tudor Ambarus wrote: > > > > > > > > > > > > > > > > > > > > > On 12/9/25 11:56 AM, Francois Berder wrote: > > > > > > > > Free buf if spi_flash_read_dm fails. > > > > > > > > > > > > > > > > Signed-off-by: Francois Berder <[email protected]> > > > > > > > > --- > > > > > > > > drivers/mtd/spi/sf_bootdev.c | 4 +++- > > > > > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > diff --git a/drivers/mtd/spi/sf_bootdev.c > > > > > > > > b/drivers/mtd/spi/sf_bootdev.c > > > > > > > > index 017a74a3016..720faf51a1f 100644 > > > > > > > > --- a/drivers/mtd/spi/sf_bootdev.c > > > > > > > > +++ b/drivers/mtd/spi/sf_bootdev.c > > > > > > > > @@ -35,8 +35,10 @@ static int sf_get_bootflow(struct udevice > > > > > > > > *dev, struct bootflow_iter *iter, > > > > > > > > > > > > > > > > ret = spi_flash_read_dm(sf, env_get_hex("script_offset_f", 0), > > > > > > > > size, buf); > > > > > > > > - if (ret) > > > > > > > > + if (ret) { > > > > > > > > + free(buf); > > > > > > > > return log_msg_ret("cmd", -EINVAL); > > > > > > > > + } > > > > > > > > > > > > > > > > > > > > > can we use a devm alloc method to get rid of the extra free > > > > > > > handling? > > > > > > > > > > > > No, because this isn't the kernel and devm alloc doesn't always free > > > > > > here. Making that no longer be the case (always freeing) is on my > > > > > > list > > > > > > to look at doing (drivers that use devm alloc always > > > > > > implys/select'ing > > > > > > DEVRES). > > > > > > > > > > We've since changed so that devm alloc does behave in U-Boot the same > > > > > as > > > > > the kernel (except in xPL phases), so Tudor's suggestion is a good > > > > > one. > > > > > > > > I still like the idea of having this patch so that others don't hit > > > > it. Does it show up as a leak on any code-analysis? > > > > > > I'm unsure what you're suggesting. If we can switch these to devm alloc > > > functions now that we match the expected API of the linux kernel > > > (outside of xPL where we don't care about leaks) we don't need to > > > manually track frees here. > > > > Yes I understand that. But it still looks like a memory leak. Does > > coverity flag this as a memory leak? > > What we have now, or what devm alloc does?
What we have now. The code looks like it has a memory leak (without the free()) but I suspect you are saying that devres doesn't look like an allocation - that would be devm_kmalloc(), right? So I suspect coverity just ignores that since it isn't called malloc(), or it knows it is a special case. > > > Also, most boards do in fact leak, if this is correct: > > > > ./tools/qconfig.py -f DEVRES DM_DEVICE_REMOVE > > 132 matches > > That is not, no: > $ ./tools/qconfig.py -f DEVRES DM_DEVICE_REMOVE > 1389 matches OK, I wonder why so many boards enable these, given the code-size impact? - Simon
