Am Dienstag, 21. Oktober 2014, 16:49:12 schrieb Yousong Zhou: > On 21 October 2014 16:17, Pär Karlsson <feino...@gmail.com> wrote: > > Yes, you are right, of course. Looking through the original implementation > > again, it seems water tight. clang probably complains about the > > uninitialized values above argcount in saved_lengths[], that are never > > reached. > > > > The precalculated strlen:s saved is likely only an optimization(?) > > attempt, > > I suppose. > > Yes. Grepping through the code shows that currently there is no > invocation of concat_strings() having more than 5 arguments. > > > Still, it seems wasteful to set up two complete loops with va_arg, and > > considering what this function actually does, I wonder if not s(n)printf > > should be used instead of this function? :-) > > I think concat_strings() is more tight and readable than multiple > strlen() + malloc() + snprintf(). > > Regards. > > yousong
Reading the answers here tells me, something is wrong with this function. There is misunderstanding / misinterpretation regarding the code. Not only the developers here are affected, but also clang (version 3.3 upto 3.6). Here is my approach - introducing a helper function (strlcpy, which exists in BSD for a long time). It seems very straight forward to me. How you like it ? char * concat_strings (const char *str0, ...) { va_list args; const char *arg; size_t length = 0, pos = 0; char *s; if (!str0) return NULL; va_start (args, str0); for (arg = str0; arg; arg = va_arg (args, const char *)) length += strlen(arg); va_end (args); s = xmalloc (length + 1); va_start (args, str0); for (arg = str0; arg; arg = va_arg (args, const char *)) pos += strlcpy(s + pos, arg, length - pos + 1); va_end (args); return s; } strlcpy() can often be used as replacement for len = snprintf(buf,"%s",string); but is ways faster. FYI, my strlcpy() code is #ifndef HAVE_STRLCPY size_t strlcpy (char *dst, const char *src, size_t size) { const char *old = src; /* Copy as many bytes as will fit */ if (size) { while (--size) { if (!(*dst++ = *src++)) return src - old - 1; } *dst = 0; } while (*src++); return src - old - 1; } #endif BTW, I already tested this in my local copy of Wget. Passes all tests (as expected ;-) Tim
signature.asc
Description: This is a digitally signed message part.