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

[Arjen de Korte]

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.