From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Mon, 10 May 2021 10:28:08 -0600 Subject: [PATCH v2 01/50] lib: Add memdup() In-Reply-To: References: <20210506142438.1310977-1-sjg@chromium.org> <20210506082420.v2.1.I1d417387eb1e7273b536017f4a8920fc4e2369a9@changeid> <20210506170739.4tz7yumpfp2kfli4@ti.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Rasmus, On Mon, 10 May 2021 at 03:00, Rasmus Villemoes wrote: > > On 06/05/2021 19.41, Simon Glass wrote: > > Hi Pratyush, > > > > On Thu, 6 May 2021 at 10:07, Pratyush Yadav wrote: > >> > >> On 06/05/21 08:23AM, Simon Glass wrote: > >>> Add a function to duplicate a memory region, a little like strdup(). > >>> > >>> Signed-off-by: Simon Glass > >>> --- > >>> > >>> Changes in v2: > >>> - Add a patch to introduce a memdup() function > >>> > >>> include/linux/string.h | 13 +++++++++++++ > >>> lib/string.c | 13 +++++++++++++ > >>> test/lib/string.c | 32 ++++++++++++++++++++++++++++++++ > >>> 3 files changed, 58 insertions(+) > >>> > >>> diff --git a/include/linux/string.h b/include/linux/string.h > >>> index dd255f21633..3169c93796e 100644 > >>> --- a/include/linux/string.h > >>> +++ b/include/linux/string.h > >>> @@ -129,6 +129,19 @@ extern void * memchr(const void *,int,__kernel_size_t); > >>> void *memchr_inv(const void *, int, size_t); > >>> #endif > >>> > >>> +/** > >>> + * memdup() - allocate a buffer and copy in the contents > >>> + * > >>> + * Note that this returns a valid pointer even if @len is 0 > >> > >> I'm uneducated about U-Boot's memory allocator. But I wonder how it > >> returns a valid pointer even on 0 length allocations. What location does > >> it point to? What are users expected to do with that pointer? They > >> obviously can't read/write to it since it is supposed to be a 0 byte > >> long allocation. If another positive length allocation happens before > >> the said pointer is freed, will it point to the same memory location? If > >> not, isn't the 0-length pointer actually at least a 1-length pointer? > > > > I think it is just a 0-length pointer and that the only thing you can > > do with it is call free(). > > > > I am certainly no expert on this sort of thing though. It seems that > > some implementations return NULL for a zero size, some return a valid > > pointer which can be passed to free(). > > It's implementation-defined, which means that one cannot know for > certain that a given malloc implementation won't return NULL for a > request of 0 bytes. The linux kernel solved that problem by introducing This function is for U-Boot board code. Note that the C-library malloc() is used for tools and we don't run these tests using that, so it is safe to assume that it is the U-Boot malloc() that is used in all cases where this function is used. > ZERO_SIZE_PTR which is basically just (void*)16L or something like that > - that way callers don't have to write their "did the allocation > succeed" test in the ugly > > if (!p && size != 0) > error_out; > > way. Of course kfree() must then accept that in addition to NULL, but > it's not really more expensive to have that early nop check be > > if ((unsigned long)ptr <= 16) > return; > > instead of > > if (!ptr) > return; > > > "man malloc" says > > RETURN VALUE > The malloc() and calloc() functions return a pointer to the > allocated memory, which is suitably aligned for any built-in type. On > error, these functions > return NULL. NULL may also be returned by a successful call to > malloc() with a size of zero, or by a successful call to calloc() with > nmemb or size equal > to zero. > > > Anyway, I don't think this helper should be put in string.c - it needs > to be in some C file that's easily compiled for both board, sandbox and > host tools (for the latter probably via the "tiny one-line wrapper that > just includes the whole real C file"). I see there's linux_string.c already. I'm not a big fan of adding a function that can be used from tools as well as U-Boot. Sandbox just uses the U-Boot libraries, so far as possible. Only a few files even compile with the host C libraries (e.g. os.c). So if we want this functionality in the host tools, I suggest we add it into the tools dir somewhere. Regards, SImon