If bread returns an error there is no bp to brelse. One of these changes was taken from NetBSD commit 0a62dad69f62 ("This works well enough to populate..."), the rest were found by looking for the same pattern. Sponsored by: The FreeBSD Foundation
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
usr.sbin/makefs/ffs/ffs_alloc.c | ||
---|---|---|
308 | this is the one that came from NetBSD |
Comment Actions
I wonder if we can use a https://github.com/coccinelle/coccinelle script to find all of these :)
bread can't return an error, though. all paths go through return (0) or err(1.,,,)
and bp is allocated unconditionally (with a call to exit it if the malloc fails). So if
we did start returning errors for bread, we'd have to make sure we brelse there.
Comment Actions
So if we did start returning errors for bread, we'd have to make sure we brelse there.
Yeah; kernel bread does:
/* * Attempt to initiate asynchronous I/O on read-ahead blocks. */ breada(vp, rablkno, rabsize, cnt, cred, flags, ckhashfunc); rv = 0; if (readwait) { rv = bufwait(bp); if (rv != 0) { brelse(bp); *bpp = NULL; } } return (rv);
so if we're going to start returning errors from our bread we'll want to do the internal brelse as well.
Comment Actions
For reference, NetBSD commit is:
commit cb2aeaa9b62f6985be27797724e2f66c74dca670 Author: christos <christos@NetBSD.org> Date: Mon Mar 13 22:17:24 2023 +0000 Don't brelse() if bread() fails. The kernel does this for us. Our bread() implementation just exits on failure, but if it didn't we would double-free. From Ed Maste (https://reviews.freebsd.org/D39069)