Hi,
On 27.10.22 08:56, Sascha Hauer wrote:
> On Wed, Oct 26, 2022 at 08:42:02AM +0200, 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.
>
> Rather than fixing a just introduced function I would expect a patch
> introducing strjoin() and then another one converting the time command
> over to use it.
What difference does it make?
>> +void *mempcpy(void *dest, const void *src, size_t count)
>> +{
>> + return memcpy(dest, src, count) + count;
>> +}
>> +EXPORT_SYMBOL(mempcpy);
>>
>> #ifndef __HAVE_ARCH_MEMMOVE
>> /**
>> @@ -943,7 +948,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);
>> @@ -951,12 +956,14 @@ char *strjoin(const char *separator, char **arr, size_t arrlen)
>> for (i = 0; i < arrlen; i++)
>> len += strlen(arr[i]) + separatorlen;
>>
>> - buf = xzalloc(len);
>> + p = buf = xmalloc(len);
>>
>> for (i = 0; i < arrlen; i++) {
>> - strcat(buf, arr[i]);
>> - strcat(buf, separator);
>> + p = stpcpy(p, arr[i]);
>> + p = mempcpy(p, separator, separatorlen);
>> }
>>
>> + p[-separatorlen] = '\0';
>
> That's a bit hard to read. How about:
>
> for (i = 0; i < arrlen; i++) {
> p = stpcpy(p, arr[i]);
> if (i < arrlen - 1)
> p = stpcpy(p, separator);
> }
>
> That would also allow you to optimize the allocated buffer size above.
I like my version more. I dislike these once-only checks inside loops.
Cheers,
Ahmad
>
> Sascha
>
--
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 |
