"Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
> index 1f8d8934cc9..0af18d88825 100644
> --- a/compat/win32/syslog.c
> +++ b/compat/win32/syslog.c
> @@ -44,6 +44,7 @@ void syslog(int priority, const char *fmt, ...)
>
> while ((pos = strstr(str, "%1")) != NULL) {
> size_t offset = pos - str;
> + char *new_pos;
> char *oldstr = str;
> str = realloc(str, st_add(++str_len, 1));
> if (!str) {
> @@ -51,9 +52,9 @@ void syslog(int priority, const char *fmt, ...)
> warning_errno("realloc failed");
> return;
> }
> - pos = str + offset;
> - memmove(pos + 2, pos + 1, strlen(pos));
> - pos[1] = ' ';
> + new_pos = str + offset;
> + memmove(new_pos + 2, new_pos + 1, strlen(new_pos));
> + new_pos[1] = ' ';
> }
Heh, after this sequence
pos = strstr(str, ...);
str = realloc(str, ...);
pos = str + something;
the compiler warns about use of pos[] as if resetting of pos based
on the new value of str[] did not even exist?
Discovering a working work-around must have been a pain.
If I were writing this, I'd probably
* avoid re-scanning "str" from the beginning in every iteration;
* possibly avoid reallocating for %1 by first scanning and counting
the problematic "%1";
* also possibly give a deeper thought to see if "%1" -> "% 1" is
necessary. We are munging the original string anyway---in the
output that has "% 1", you cannot tell if the original was "%1"
that was munged by this code, or it was "% 1" from the beginning.
In other words, we are already willing to lose information.
Perhaps we can find a different replacement sequence that is
still 2-byte-wide and that is unlikely to appear in the original,
eliminating the need for reallocation altogether?
But none of that is a problem with this patch.
Will queue. Thanks.
