Hi Tom, On Tue, 1 Apr 2025 at 10:50, Tom Rini <tr...@konsulko.com> wrote: > > On Wed, Apr 02, 2025 at 04:46:23AM +1300, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 1 Apr 2025 at 06:42, Tom Rini <tr...@konsulko.com> wrote: > > > > > > On Wed, Mar 19, 2025 at 03:37:55PM +0100, Simon Glass wrote: > > > > > > > Using an abuf for this function simplifies returning the size and also > > > > makes it easier to free memory afterwards. Update the API and callers. > > > > > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > > > --- > > > > > > > > boot/bootmeth-uclass.c | 19 ++++++++++--------- > > > > fs/fs.c | 25 +++++++++++-------------- > > > > include/fs.h | 8 +++++--- > > > > 3 files changed, 26 insertions(+), 26 deletions(-) > > > > > > So we grow platforms by ~200 bytes: > > > sama7g54_curiosity_nandflash: all +204 text +204 > > > u-boot: add: 6/0, grow: 2/0 bytes: 204/0 (204) > > > function old new > > > delta > > > abuf_realloc - 76 > > > +76 > > > abuf_uninit_move - 42 > > > +42 > > > memdup - 28 > > > +28 > > > abuf_uninit - 24 > > > +24 > > > fs_read_alloc 96 106 > > > +10 > > > fs_load_alloc 114 124 > > > +10 > > > abuf_init - 10 > > > +10 > > > abuf_addr - 4 > > > +4 > > > > > > To move away from standard buffer usage and unwinding to move to > > > something homegrown instead. I am not a fan of growing using abuf here. > > > When it was introduced in: > > > > > > commit 67bc59df05331eaac56cd0a00219d1386130aee2 > > > Author: Simon Glass <s...@chromium.org> > > > Date: Sat Sep 25 07:03:07 2021 -0600 > > > > > > Add support for an owned buffer > > > > > > It sounded like something for some special cases. Not something to be > > > used everywhere to be different. > > > > From what I can tell this is a one-off hit that I believe is worth > > taking at some point. However I haven't seen a code-size reduction > > yet, so I can understand your reluctance. > > I'm not sure why it's worth taking. I'm not sure that abuf is worth > pushing in to every place we allocate a buffer for strings. If someone > wants to put in effort to make our handling of strings better/safer/more > secure we should look to all of the work Kees Cook (et al) did in the > kernel and borrow that, not invent something new and U-Boot specific.
This isn't about strings; it's about buffers. A buffer which can be allocated or not, which can be resized and can be passed around without worrying about pointers and sizes and so on. It also allows one less parameter to be passed, although of course it is a pointer to a struct, so it doesn't always make the code smaller than passing two parameters. I've had another look through this and I believe this is a step forward in terms of code safety, if nothing else. I've also found a way to reduce the impact on your sample board to 104 bytes. This is largely a one-off cost, so move features converting to abuf will benefit from this without further code-size impact. Regards, Simon
