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