Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
- From
Erik Faye-Lund <kusmabite@gmail.com>
- Date
- Aug 20, 2013, 22:26 UTC
- Message-ID
- <CABPQNSZSXgvLw_i25em38B70EgtZrv9iQh8OaCJzYRCk2vjZXQ@mail.gmail.com>
- In-Reply-To
- <87eh9op5gq.fsf@igel.home>
On Tue, Aug 20, 2013 at 8:44 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
Show 23 quoted lines
> Erik Faye-Lund <kusmabite@gmail.com> writes:
>
>> diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
>> index d015e43..0641f4e 100644
>> --- a/compat/win32/syslog.c
>> +++ b/compat/win32/syslog.c
>> @@ -43,11 +43,14 @@ void syslog(int priority, const char *fmt, ...)
>> va_end(ap);
>>
>> while ((pos = strstr(str, "%1")) != NULL) {
>> - str = realloc(str, ++str_len + 1);
>> - if (!str) {
>> + char *tmp = realloc(str, ++str_len + 1);
>> + if (!tmp) {
>> warning("realloc failed: '%s'", strerror(errno));
>> + free(str);
>> return;
>> }
>> + pos = tmp + (pos - str);
>
> Pedantically, this is undefined (uses of both pos and str may trap after
> realloc has freed the original pointer), it is better to calculate the
> difference before calling realloc.I don't see how it's undefined. It's using the memory that 'pos' *points to* that is undefined, no? The difference between 'pos' and 'str' should still be the same, it's not like realloc somehow magically updates 'pos'...