[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and pote
From: |
Tim Rühsen |
Subject: |
Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings |
Date: |
Thu, 23 Oct 2014 22:01:42 +0200 |
User-agent: |
KMail/4.14.1 (Linux/3.16-2-amd64; KDE/4.14.1; x86_64; ; ) |
Am Dienstag, 21. Oktober 2014, 16:49:12 schrieb Yousong Zhou:
> On 21 October 2014 16:17, Pär Karlsson <address@hidden> 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.
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, (continued)
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Pär Karlsson, 2014/10/20
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Pär Karlsson, 2014/10/20
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Yousong Zhou, 2014/10/20
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Yousong Zhou, 2014/10/20
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Micah Cowan, 2014/10/21
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Pär Karlsson, 2014/10/21
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Micah Cowan, 2014/10/21
Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Yousong Zhou, 2014/10/20
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Pär Karlsson, 2014/10/21
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Yousong Zhou, 2014/10/21
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings,
Tim Rühsen <=
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Pär Karlsson, 2014/10/24
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Tim Rühsen, 2014/10/24
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Pär Karlsson, 2014/10/25
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Tim Rühsen, 2014/10/27
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Pär Karlsson, 2014/10/27
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Tim Ruehsen, 2014/10/29