[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: |
Yousong Zhou |
Subject: |
Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings |
Date: |
Tue, 21 Oct 2014 10:22:44 +0800 |
Hi, Pär
On 17 October 2014 03:50, Pär Karlsson <address@hidden> wrote:
> Hi, I fould a potential gotcha when playing with clang's code analysis tool.
>
> The concat_strings function silently stopped counting string lengths when
> given more than 5 arguments. clang warned about potential garbage values in
> the saved_lengths array, so I redid it with this approach.
After taking a closer look, I guess the old implementation is fine.
saved_length[] is used as a buffer for lengths of the first 5
arguments and there is a bound check with its length. Maybe it's a
false-positive from clang tool?
Sorry for the noise...
Regards.
yousong
>
> All tests working ok with this patch.
>
> This is my first patch to this list, by the way. I'd be happy to help out
> more in the future.
>
> Best regards,
>
> /Pär Karlsson, Sweden
>
> ----
>
> commit 2d855670e0e1fbe578506b376cdd40b0e465d3ef
> Author: Pär Karlsson <address@hidden>
> Date: Thu Oct 16 21:41:36 2014 +0200
>
> Updated ChangeLog
>
> diff --git a/src/ChangeLog b/src/ChangeLog
> index 1c4e2d5..1e39475 100644
> --- a/src/ChangeLog
> +++ b/src/ChangeLog
> @@ -1,3 +1,8 @@
> +2014-10-16 Pär Karlsson <address@hidden>
> +
> + * utils.c (concat_strings): fixed arbitrary limit of 5 arguments to
> + function
> +
> 2014-05-03 Tim Ruehsen <address@hidden>
>
> * retr.c (retrieve_url): fixed memory leak
>
> commit 1fa9ff274dcb6e5a2dbbbc7d3fe2f139059c47f1
> Author: Pär Karlsson <address@hidden>
> Date: Wed Oct 15 00:00:31 2014 +0200
>
> Generalized concat_strings argument length
>
> The concat_strings function seemed arbitrary to only accept a maximum
> of 5 arguments (the others were silently ignored).
>
> Also it had a potential garbage read of the values in the array.
> Updated with xmalloc/xrealloc/free
>
> diff --git a/src/utils.c b/src/utils.c
> index 78c282e..93c9ddc 100644
> --- a/src/utils.c
> +++ b/src/utils.c
> @@ -356,7 +356,8 @@ char *
> concat_strings (const char *str0, ...)
> {
> va_list args;
> - int saved_lengths[5]; /* inspired by Apache's apr_pstrcat */
> + size_t psize = sizeof(int);
> + int *saved_lengths = xmalloc (psize);
> char *ret, *p;
>
> const char *next_str;
> @@ -370,8 +371,8 @@ concat_strings (const char *str0, ...)
> for (next_str = str0; next_str != NULL; next_str = va_arg (args, char *))
> {
> int len = strlen (next_str);
> - if (argcount < countof (saved_lengths))
> - saved_lengths[argcount++] = len;
> + saved_lengths[argcount++] = len;
> + xrealloc(saved_lengths, psize * argcount);
> total_length += len;
> }
> va_end (args);
> @@ -393,7 +394,7 @@ concat_strings (const char *str0, ...)
> }
> va_end (args);
> *p = '\0';
> -
> + free(saved_lengths);
> return ret;
> }
> ^L
- [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Pär Karlsson, 2014/10/16
- 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/17
- Re: [Bug-wget] [PATCH] Small fix for limited number of strings (and potential garbage value) in arguments to concat_strings, Giuseppe Scrivano, 2014/10/19
- 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 <=
- 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, 2014/10/23
- 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