On 2024/12/13 12:57, Kent Overstreet wrote:
On Fri, Dec 13, 2024 at 12:03:25PM +0800, Hongbo Li wrote:
Add printbuf_err helper to return standard error when
allocation failure.
v3:
- Remove extra sign-off and rebase the code.
v2:
https://lore.kernel.org/linux-bcachefs/[email protected]/
- add printbuf_err helper and use it.
v1: https://lore.kernel.org/all/[email protected]/
Hongbo Li (3):
bcachefs: add printbuf_err helper
bcachefs: avoid copy from NULL printbuf when allocation failure
bcachefs: use printbuf_err helper
fs/bcachefs/fs.c | 9 +++++++--
fs/bcachefs/opts.c | 7 +------
fs/bcachefs/printbuf.h | 5 +++++
fs/bcachefs/sysfs.c | 4 +---
4 files changed, 14 insertions(+), 11 deletions(-)
--
2.34.1
A thought.
I haven't been liking the idea of adding new error checks all over the
place (and if we interpret this strictly, they'd need to be added
_everywhere_) - a lot of pain for very little gain, considering that in
practice small allocations will never fail.
And most printbuf uses wouldn't be doing anything with an extra -ENOMEM
check, the few that need it do check for it explicitly.
How about adjusting the strategies of resize to decrease the realloc in
bch2_prt_printf (if new size is closed to the original size, it may
invoke realloc again.)? Or add some limited log for allocation failure.
Anyway, may be the check for allocation is necessary.
But this is something that we should handle, because we could be using
a printbuf in an atomic context, or the debate over memory allocator
failure behaviour could change (unlikely, but) - or we could be trying
to print a string for which the initial allocation was over the size
that's guaranteed not to fail by the allocator.
So we want a way to make getting the string safe, without affecting the
ergonomics so much.
So...
What if we had a printbuf_str() method for getting the buffer, instead
of accessing printbuf.buf directly?
It works well for the following step, but it may ignore the memory
allocator failure cases if lack of the checker. And this makes it
difficult for debugging.
Then instead of returning a NULL pointer, if printbuf.buf is null it
could just return a pointer to the zero page (which conveniently is a
null terminated zero length string, for our purposes).
