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@xxxxxxxxxxxxxx> >> --- >> Originally posted at: https://lore.barebox.org/barebox/20221027073334.GS6702@xxxxxxxxxxxxxx/ > > 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@xxxxxxxxxxxxxx> >> --- >> 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 |
