[PATCH v2 01/50] lib: Add memdup()

[Rasmus Villemoes]

On 06/05/2021 19.41, Simon Glass wrote:
> Hi Pratyush,
> 
> On Thu, 6 May 2021 at 10:07, Pratyush Yadav <p.yadav at ti.com> 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 <sjg at chromium.org>
>>> ---
>>>
>>> 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
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.

Rasmus