[PATCH v2 2/7] common: Tidy up how malloc() is inited

[Heinrich Schuchardt]

Am 11. Oktober 2024 23:40:26 MESZ schrieb Simon Glass <sjg at chromium.org>:
>The call to malloc() is a bit strange. The naming of the arguments
>suggests that an address is passed, but in fact it is a pointer, at
>least in the board_init_r() function and SPL equivalent.


Please, do not push sandbox only code deeper into malloc.

We should strive to get rid of the sandbox only virtual address space whereever we can.

Best regards

Heinrich

>
>Update it to work as described. Add a function comment as well.
>
>Note that this does adjustment does not extend into the malloc()
>implementation itself, apart from changing mem_malloc_init(), since
>there are lots of casts and pointers and integers are used
>interchangeably.
>
>Signed-off-by: Simon Glass <sjg at chromium.org>
>---
>
>(no changes since v1)
>
> common/board_r.c  | 3 +--
> common/dlmalloc.c | 8 +++++---
> common/spl/spl.c  | 4 +---
> include/malloc.h  | 8 ++++++++
> 4 files changed, 15 insertions(+), 8 deletions(-)
>
>diff --git a/common/board_r.c b/common/board_r.c
>index 1acad069d92..60eba0942d8 100644
>--- a/common/board_r.c
>+++ b/common/board_r.c
>@@ -204,8 +204,7 @@ static int initr_malloc(void)
> 	 */
> 	start = gd->relocaddr - TOTAL_MALLOC_LEN;
> 	gd_set_malloc_start(start);
>-	mem_malloc_init((ulong)map_sysmem(start, TOTAL_MALLOC_LEN),
>-			TOTAL_MALLOC_LEN);
>+	mem_malloc_init(start, TOTAL_MALLOC_LEN);
> 	return 0;
> }
> 
>diff --git a/common/dlmalloc.c b/common/dlmalloc.c
>index 1ac7ce3f43c..cc4d3a0a028 100644
>--- a/common/dlmalloc.c
>+++ b/common/dlmalloc.c
>@@ -16,6 +16,8 @@
> #include <asm/global_data.h>
> 
> #include <malloc.h>
>+#include <mapmem.h>
>+#include <string.h>
> #include <asm/io.h>
> #include <valgrind/memcheck.h>
> 
>@@ -598,9 +600,9 @@ void *sbrk(ptrdiff_t increment)
> 
> void mem_malloc_init(ulong start, ulong size)
> {
>-	mem_malloc_start = start;
>-	mem_malloc_end = start + size;
>-	mem_malloc_brk = start;
>+	mem_malloc_start = (ulong)map_sysmem(start, size);
>+	mem_malloc_end = mem_malloc_start + size;
>+	mem_malloc_brk = mem_malloc_start;
> 
> #ifdef CONFIG_SYS_MALLOC_DEFAULT_TO_INIT
> 	malloc_init();
>diff --git a/common/spl/spl.c b/common/spl/spl.c
>index c13b2b8f714..fc8b38862e8 100644
>--- a/common/spl/spl.c
>+++ b/common/spl/spl.c
>@@ -679,9 +679,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> 	spl_set_bd();
> 
> 	if (IS_ENABLED(CONFIG_SPL_SYS_MALLOC)) {
>-		mem_malloc_init((ulong)map_sysmem(SPL_SYS_MALLOC_START,
>-						  SPL_SYS_MALLOC_SIZE),
>-				SPL_SYS_MALLOC_SIZE);
>+		mem_malloc_init(SPL_SYS_MALLOC_START, SPL_SYS_MALLOC_SIZE);
> 		gd->flags |= GD_FLG_FULL_MALLOC_INIT;
> 	}
> 	if (!(gd->flags & GD_FLG_SPL_INIT)) {
>diff --git a/include/malloc.h b/include/malloc.h
>index 07d3e90a855..9e0be482416 100644
>--- a/include/malloc.h
>+++ b/include/malloc.h
>@@ -981,6 +981,14 @@ extern ulong mem_malloc_start;
> extern ulong mem_malloc_end;
> extern ulong mem_malloc_brk;
> 
>+/**
>+ * mem_malloc_init() - Set up the malloc() pool
>+ *
>+ * Sets the region of memory to be used for all future calls to malloc(), etc.
>+ *
>+ * @start: Start address
>+ * @size: Size in bytes
>+ */
> void mem_malloc_init(ulong start, ulong size);
> 
> #ifdef __cplusplus