Re: [Minios-devel] [UNIKRAFT PATCH v2 02/10] lib/nolibc: Add strdup function

[Yuri Volchkov <yuri.volchkov@xxxxxxxxx>]

Ok, I must agree here.

Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:

> SIZE_MAX is the biggest value of size_t and the datatype size_t is sized 
> for storing the theoretical maximum possible size of an object on your 
> architecture (size_t is 32bits on 32bits and 64bits on 64bits). So, we 
> are fully compliant by using strndup() with SIZE_MAX to implement 
> strdup(). There is no truncation possible. The only thing that could 
> happen is that you stop instead of having an infinite loop if you have 
> nowhere a 0-byte in your fully mapped 16EB address space. If you want to 
> have this as your defined behavior you are still free to use newlib or 
> musl instead of nolibc.
>
> I suggested to prefer strndup() in nolibc because it should be anyways 
> be best practice to use this variant instead in your code. You could 
> save implementing strdup() by wrapping it around strndup() as I suggested.
>
> On 28.08.2018 14:33, Yuri Volchkov wrote:
>> Hey,
>> 
>> that would be incorrect behavior of strdup. There is no mention about
>> any length limit in the "man strdup". I checked the musl's
>> implementation - it does not have any limitation either.
>> 
>> So in case of improbable event that the source string is longer then
>> SIZE_MAX, the duplicate will be a truncated copy. Happy debugging :)
>> 
>> - Yuri
>> 
>> Simon Kuenzer <simon.kuenzer@xxxxxxxxx> writes:
>> 
>>> Hey,
>>>
>>> On 23.08.2018 12:59, Costin Lupu wrote:
>>>> Shamelessly taken from Mini-OS.
>>>>
>>>> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
>>>> ---
>>>>    lib/nolibc/include/string.h |  1 +
>>>>    lib/nolibc/string.c         | 17 +++++++++++++++++
>>>>    2 files changed, 18 insertions(+)
>>>>
>>>> diff --git a/lib/nolibc/include/string.h b/lib/nolibc/include/string.h
>>>> index 677f528..8674c77 100644
>>>> --- a/lib/nolibc/include/string.h
>>>> +++ b/lib/nolibc/include/string.h
>>>> @@ -57,6 +57,7 @@ size_t strlen(const char *str);
>>>>    const char *strchr(const char *str, int c);
>>>>    int strncmp(const char *str1, const char *str2, size_t len);
>>>>    int strcmp(const char *str1, const char *str2);
>>>> +char *strdup(const char *str);
>>>>    
>>>>    #ifdef __cplusplus
>>>>    }
>>>> diff --git a/lib/nolibc/string.c b/lib/nolibc/string.c
>>>> index bf89106..bf4ab50 100644
>>>> --- a/lib/nolibc/string.c
>>>> +++ b/lib/nolibc/string.c
>>>> @@ -33,6 +33,7 @@
>>>>     * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>>>>     */
>>>>    
>>>> +#include <stdlib.h>
>>>>    #include <stdint.h>
>>>>    #include <string.h>
>>>>    #include <limits.h>
>>>> @@ -166,3 +167,19 @@ int strcmp(const char *str1, const char *str2)
>>>>    
>>>>            return __res;
>>>>    }
>>>> +
>>>> +char *strdup(const char *str)
>>>> +{
>>>> +  char *__res;
>>>> +  int __len;
>>>> +
>>>> +  __len = strlen(str);
>>>> +
>>>> +  __res = malloc(__len + 1);
>>>> +  if (!__res)
>>>> +          return NULL;
>>>> +
>>>> +  memcpy(__res, str, __len + 1);
>>>> +
>>>> +  return __res;
>>>> +}
>>>>
>>>
>>> Could you provide an strndup() instead and make strdup() use it? This
>>> way we would cover both libc variants with one patch.
>>>
>>> I mean, you would do something like:
>>>
>>> char *strdup(const char *str)
>>> {
>>>     return strndup(str, SIZE_MAX);
>>> }
>>>
>>> Cheers,
>>>
>>> Simon
>> 

-- 
Yuri Volchkov
Software Specialist

NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel