[PATCH 047/112] string: reduce strjoin runtime, drop trailing separator

[Ahmad Fatoum]

On 08.01.24 08:11, Sascha Hauer wrote:
> On Wed, Jan 03, 2024 at 07:12:07PM +0100, Ahmad Fatoum wrote:
>> The implementation of strjoin is a bit suboptimal. The destination
>> string is traversed from the beginning due to strcat and we have a
>> left-over separator at the end, while it should only be in-between.
>>
>> Fix this.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum at pengutronix.de>
>> ---
>> Originally posted at: https://lore.barebox.org/barebox/20221027073334.GS6702@pengutronix.de/
> 
> Once again I ended up reviewing a suboptimal version of strjoin() first
> just to find my potential comments addressed in the next patch. So my
> comment to the last version still stands: Please implement a good
> version of strjoin() first and then switch over to use it.

The first patch just moves code around. I find it completely valid to move
code before doing changes to it...

> 
>>
>> Changes:
>>   - remove if statemnt in loop (Sascha)
>> Signed-off-by: Ahmad Fatoum <a.fatoum at pengutronix.de>
>> ---
>>  lib/string.c | 15 ++++++++++-----
>>  1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/string.c b/lib/string.c
>> index d8e5edd40648..695e50bc8fc1 100644
>> --- a/lib/string.c
>> +++ b/lib/string.c
>> @@ -1005,7 +1005,7 @@ char *strjoin(const char *separator, char **arr, size_t arrlen)
>>  {
>>  	size_t separatorlen;
>>  	int len = 1; /* '\0' */
>> -	char *buf;
>> +	char *buf, *p;
>>  	int i;
>>  
>>  	separatorlen = strlen(separator);
>> @@ -1013,13 +1013,18 @@ char *strjoin(const char *separator, char **arr, size_t arrlen)
>>  	for (i = 0; i < arrlen; i++)
>>  		len += strlen(arr[i]) + separatorlen;
> 
> Not that it matters much for memory usage, but for consistency you could
> drop the final separatorlen just like you did for copying the strings
> below.
> 
> Sascha
> 
>>  
>> -	buf = xzalloc(len);
>> +	if (!arrlen)
>> +		return xzalloc(1);
>>  
>> -	for (i = 0; i < arrlen; i++) {
>> -		strcat(buf, arr[i]);
>> -		strcat(buf, separator);
>> +	p = buf = xmalloc(len);
>> +
>> +	for (i = 0; i < arrlen - 1; i++) {
>> +		p = stpcpy(p, arr[i]);
>> +		p = mempcpy(p, separator, separatorlen);
>>  	}
>>  
>> +	stpcpy(p, arr[i]);
>> +
>>  	return buf;
>>  }
>>  EXPORT_SYMBOL(strjoin);
>> -- 
>> 2.39.2
>>
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |