[LEDE-DEV] [PATCH] libubox: replace strtok with strsep

[John Crispin]

On 18/12/2016 17:29, Arjen de Korte wrote:
> Citeren John Crispin <john at phrozen.org>:
> 
>> On 16/12/2016 11:10, Felix Fietkau wrote:
>>> On 2016-12-16 10:58, John Crispin wrote:
>>>>
>>>>
>>>> On 14/12/2016 06:43, Rosen Penev wrote:
>>>>> strsep is re-entrant whereas strtok not necessarily so.
>>>>>
>>>>> Signed-off by: Rosen Penev <rosenp at gmail.com>
>>>>> ---
>>>>>  ulog.c | 4 ++--
>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/ulog.c b/ulog.c
>>>>> index 296605d..985d366 100644
>>>>> --- a/ulog.c
>>>>> +++ b/ulog.c
>>>>> @@ -39,8 +39,8 @@ static const char *ulog_default_ident(void)
>>>>>      if ((self = fopen("/proc/self/status", "r")) != NULL) {
>>>>>          while (fgets(line, sizeof(line), self)) {
>>>>>              if (!strncmp(line, "Name:", 5)) {
>>>>> -                strtok(line, "\t\n");
>>>>> -                p = strtok(NULL, "\t\n");
>>>>> +                strsep(&line, "\t\n");
>>>>> +                p = strsep(&line, "\t\n");
>>>>
>>>> Hi,
>>>>
>>>> i agree that using strsep() is a nice api however i fail to see why the
>>>> original code would fail. according to the man page it is re-entrant,
>>>> specially if called as done here. can you elaborate a bit why you think
>>>> it is not re-entrant ?
>>> If the caller of ulog() is using strtok between ulog calls, internal use
>>> of strtok can break it. I think this change is worth merging.
>>>
>>> - Felix
>>>
>>
>> ah of course, i did not thing of the caller using it, i'll merge it in
>> that case
> 
> Bad idea. Please consider using strtok_r instead (this is a reetrant
> version of strtok) like in jshn.c. Functionally, strsep and strtok will
> behave differently if there are multiple delimiters between tokens. I
> doubt it is worth it to check if this difference in behavior is OK, if
> one can use strtok_r. So if thread safety is the only concern, replace
> strtok by strtok_r (not strsep). Also, keep in mind that the first
> argument passed to strsep is mofified, so if that happens to be a
> pointer to dynamically allocated memory and you use that pointer to free
> said memory, you'll have a memory leak.
> 
> 

even better, Rosen could you please change the patch to use strtok_r
(not strsep)

	John