From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Thu, 06 Oct 2011 16:52:43 GMT
Subject: Re: [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere
Message-ID: <CABPQNSaQFFj0vP1+wuoL2=GyHZPRgCXLxXqD+5WX3Le_PyuH4w@mail.gmail.com>
In-Reply-To: <CABPQNSak_jDbNQhzMoSN=NdWmyby3xJRwED54Ck5H1Y12HoGCQ@mail.gmail.com>

On Thu, Oct 6, 2011 at 6:50 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Thu, Oct 6, 2011 at 6:14 PM, Brandon Casey <drafnel@gmail.com> wrote:
>> Ah.  Yes, you're right.  x-wrappers should not be used in syslog.c and
>> the use of strbuf's should be replaced.
>
> Good point. The patch for this looks something like this:
>
> diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
> index 42b95a9..243538c 100644
> --- a/compat/win32/syslog.c
> +++ b/compat/win32/syslog.c
> @@ -1,5 +1,4 @@
>  #include "../../git-compat-util.h"
> -#include "../../strbuf.h"
>
>  static HANDLE ms_eventlog;
>
> @@ -16,13 +15,8 @@ void openlog(const char *ident, int logopt, int facility)
>
>  void syslog(int priority, const char *fmt, ...)
>  {
> -       struct strbuf sb = STRBUF_INIT;
> -       struct strbuf_expand_dict_entry dict[] = {
> -               {"1", "% 1"},
> -               {NULL, NULL}
> -       };
>        WORD logtype;
> -       char *str;
> +       char *str, *pos;
>        int str_len;
>        va_list ap;
>
> @@ -39,11 +33,20 @@ void syslog(int priority, const char *fmt, ...)
>        }
>
>        str = malloc(str_len + 1);
> +       if (!str)
> +               return; /* no chance to report error */
> +
>        va_start(ap, fmt);
>        vsnprintf(str, str_len + 1, fmt, ap);
>        va_end(ap);
> -       strbuf_expand(&sb, str, strbuf_expand_dict_cb, &dict);
> -       free(str);
> +
> +       while ((pos = strstr(str, "%1")) != NULL) {
> +               str = realloc(str, ++str_len + 1);
> +               if (!str)
> +                       return;
> +               memmove(pos + 2, pos + 1, strlen(pos));
> +               pos[1] = ' ';
> +       }
>
>        switch (priority) {
>        case LOG_EMERG:
> @@ -66,7 +69,5 @@ void syslog(int priority, const char *fmt, ...)
>        }
>
>        ReportEventA(ms_eventlog, logtype, 0, 0, NULL, 1, 0,
> -           (const char **)&sb.buf, NULL);
> -
> -       strbuf_release(&sb);
> +           (const char **)&str, NULL);
>  }
>

...aaand this on top to avoid a leak, of course:

diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c
index 243538c..3b8e2b7 100644
--- a/compat/win32/syslog.c
+++ b/compat/win32/syslog.c
@@ -70,4 +70,5 @@ void syslog(int priority, const char *fmt, ...)

 	ReportEventA(ms_eventlog, logtype, 0, 0, NULL, 1, 0,
 	    (const char **)&str, NULL);
+	free(str);
 }