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

[Sascha Hauer <sha@pengutronix.de>]

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@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.

> 
> Changes:
>   - remove if statemnt in loop (Sascha)
> Signed-off-by: Ahmad Fatoum <a.fatoum@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 |