Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4

[Erik Faye-Lund <kusmabite@gmail.com>]

On Tue, Aug 20, 2013 at 8:44 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> 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'...