{"thread":{"id":"28606","subject":"[PATCH] mingw: avoid using strbuf in syslog","startedAt":"2011-10-06T16:57:24Z","lastAt":"2011-10-06T17:32:09Z","messageCount":3,"participants":["Erik Faye-Lund","Brandon Casey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"177058","messageId":"1317920244-6320-1-git-send-email-kusmabite@gmail.com","threadId":"28606","inReplyTo":null,"subject":"[PATCH] mingw: avoid using strbuf in syslog","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-10-06T16:57:24Z","receivedAt":"2011-10-06T16:57:24Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"strbuf can call die, which again can call syslog from git-daemon.\n\nEndless recursion is no fun; fix it by hand-rolling the logic.\n\nSigned-off-by: Erik Faye-Lund \n---\nFixes the problem Brandon pointed out...\n\n compat/win32/syslog.c | 26 ++++++++++++++------------\n 1 files changed, 14 insertions(+), 12 deletions(-)\n\ndiff --git a/compat/win32/syslog.c b/compat/win32/syslog.c\nindex 42b95a9..3b8e2b7 100644\n--- a/compat/win32/syslog.c\n+++ b/compat/win32/syslog.c\n@@ -1,5 +1,4 @@\n #include \"../../git-compat-util.h\"\n-#include \"../../strbuf.h\"\n \n static HANDLE ms_eventlog;\n \n@@ -16,13 +15,8 @@ void openlog(const char *ident, int logopt, int facility)\n \n void syslog(int priority, const char *fmt, ...)\n {\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tstruct strbuf_expand_dict_entry dict[] = {\n-\t\t{\"1\", \"% 1\"},\n-\t\t{NULL, NULL}\n-\t};\n \tWORD logtype;\n-\tchar *str;\n+\tchar *str, *pos;\n \tint str_len;\n \tva_list ap;\n \n@@ -39,11 +33,20 @@ void syslog(int priority, const char *fmt, ...)\n \t}\n \n \tstr = malloc(str_len + 1);\n+\tif (!str)\n+\t\treturn; /* no chance to report error */\n+\n \tva_start(ap, fmt);\n \tvsnprintf(str, str_len + 1, fmt, ap);\n \tva_end(ap);\n-\tstrbuf_expand(&sb, str, strbuf_expand_dict_cb, &dict);\n-\tfree(str);\n+\n+\twhile ((pos = strstr(str, \"%1\")) != NULL) {\n+\t\tstr = realloc(str, ++str_len + 1);\n+\t\tif (!str)\n+\t\t\treturn;\n+\t\tmemmove(pos + 2, pos + 1, strlen(pos));\n+\t\tpos[1] = ' ';\n+\t}\n \n \tswitch (priority) {\n \tcase LOG_EMERG:\n@@ -66,7 +69,6 @@ void syslog(int priority, const char *fmt, ...)\n \t}\n \n \tReportEventA(ms_eventlog, logtype, 0, 0, NULL, 1, 0,\n-\t (const char **)&sb.buf, NULL);\n-\n-\tstrbuf_release(&sb);\n+\t (const char **)&str, NULL);\n+\tfree(str);\n }\n-- \n1.7.6.msysgit.0.579.ga3d6f\n"},{"id":"177069","messageId":"CA+sFfMediaCnzFH6uKfVc_Bb+W+AA1nO+OdvB8vWjjrsD1kh9w@mail.gmail.com","threadId":"28606","inReplyTo":"1317920244-6320-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH] mingw: avoid using strbuf in syslog","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2011-10-06T17:27:29Z","receivedAt":"2011-10-06T17:27:29Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Thu, Oct 6, 2011 at 11:57 AM, Erik Faye-Lund wrote:\n> strbuf can call die, which again can call syslog from git-daemon.\n>\n> Endless recursion is no fun; fix it by hand-rolling the logic.\n>\n> Signed-off-by: Erik Faye-Lund \n> ---\n> Fixes the problem Brandon pointed out...\n\nActually, Johannes should get that credit. He brought up the whole\nrecursion issue.\n\n>  compat/win32/syslog.c |   26 ++++++++++++++------------\n>  1 files changed, 14 insertions(+), 12 deletions(-)\n>\n> diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c\n> index 42b95a9..3b8e2b7 100644\n> --- a/compat/win32/syslog.c\n> +++ b/compat/win32/syslog.c\n> @@ -1,5 +1,4 @@\n>  #include \"../../git-compat-util.h\"\n> -#include \"../../strbuf.h\"\n>\n>  static HANDLE ms_eventlog;\n>\n> @@ -16,13 +15,8 @@ void openlog(const char *ident, int logopt, int facility)\n>\n>  void syslog(int priority, const char *fmt, ...)\n>  {\n> -       struct strbuf sb = STRBUF_INIT;\n> -       struct strbuf_expand_dict_entry dict[] = {\n> -               {\"1\", \"% 1\"},\n> -               {NULL, NULL}\n> -       };\n>        WORD logtype;\n> -       char *str;\n> +       char *str, *pos;\n>        int str_len;\n>        va_list ap;\n>\n> @@ -39,11 +33,20 @@ void syslog(int priority, const char *fmt, ...)\n>        }\n>\n>        str = malloc(str_len + 1);\n> +       if (!str)\n> +               return; /* no chance to report error */\n> +\n\nJust above this, warning() is used to at least print a message to\nstderr if vsnprintf() fails. It could be used here, and below when\nrealloc fails.\n\n>        va_start(ap, fmt);\n>        vsnprintf(str, str_len + 1, fmt, ap);\n>        va_end(ap);\n> -       strbuf_expand(&sb, str, strbuf_expand_dict_cb, &dict);\n> -       free(str);\n> +\n> +       while ((pos = strstr(str, \"%1\")) != NULL) {\n> +               str = realloc(str, ++str_len + 1);\n> +               if (!str)\n> +                       return;\n> +               memmove(pos + 2, pos + 1, strlen(pos));\n> +               pos[1] = ' ';\n> +       }\n>\n>        switch (priority) {\n>        case LOG_EMERG:\n\n-Brandon\n"},{"id":"177073","messageId":"CABPQNSYUzmHgFO9VzHRX-or8b5h+HEXapBoiQDWRA_4KWi9nAQ@mail.gmail.com","threadId":"28606","inReplyTo":"CA+sFfMediaCnzFH6uKfVc_Bb+W+AA1nO+OdvB8vWjjrsD1kh9w@mail.gmail.com","subject":"Re: [PATCH] mingw: avoid using strbuf in syslog","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-10-06T17:32:09Z","receivedAt":"2011-10-06T17:32:09Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Oct 6, 2011 at 7:27 PM, Brandon Casey wrote:\n> On Thu, Oct 6, 2011 at 11:57 AM, Erik Faye-Lund wrote:\n>> strbuf can call die, which again can call syslog from git-daemon.\n>>\n>> Endless recursion is no fun; fix it by hand-rolling the logic.\n>>\n>> Signed-off-by: Erik Faye-Lund \n>> ---\n>> Fixes the problem Brandon pointed out...\n>\n> Actually, Johannes should get that credit.  He brought up the whole\n> recursion issue.\n>\n\nOK, I didn't spot that one. But it's not in the commit message,\nthough. Perhaps it should be?\n\n>>  compat/win32/syslog.c |   26 ++++++++++++++------------\n>>  1 files changed, 14 insertions(+), 12 deletions(-)\n>>\n>> diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c\n>> index 42b95a9..3b8e2b7 100644\n>> --- a/compat/win32/syslog.c\n>> +++ b/compat/win32/syslog.c\n>> @@ -1,5 +1,4 @@\n>>  #include \"../../git-compat-util.h\"\n>> -#include \"../../strbuf.h\"\n>>\n>>  static HANDLE ms_eventlog;\n>>\n>> @@ -16,13 +15,8 @@ void openlog(const char *ident, int logopt, int facility)\n>>\n>>  void syslog(int priority, const char *fmt, ...)\n>>  {\n>> -       struct strbuf sb = STRBUF_INIT;\n>> -       struct strbuf_expand_dict_entry dict[] = {\n>> -               {\"1\", \"% 1\"},\n>> -               {NULL, NULL}\n>> -       };\n>>        WORD logtype;\n>> -       char *str;\n>> +       char *str, *pos;\n>>        int str_len;\n>>        va_list ap;\n>>\n>> @@ -39,11 +33,20 @@ void syslog(int priority, const char *fmt, ...)\n>>        }\n>>\n>>        str = malloc(str_len + 1);\n>> +       if (!str)\n>> +               return; /* no chance to report error */\n>> +\n>\n> Just above this, warning() is used to at least print a message to\n> stderr if vsnprintf() fails.  It could be used here, and below when\n> realloc fails.\n>\n\nGood point, so this should be squashed in:\n---\n\ndiff --git a/compat/win32/syslog.c b/compat/win32/syslog.c\nindex 3b8e2b7..d015e43 100644\n--- a/compat/win32/syslog.c\n+++ b/compat/win32/syslog.c\n@@ -33,8 +33,10 @@ void syslog(int priority, const char *fmt, ...)\n \t}\n\n \tstr = malloc(str_len + 1);\n-\tif (!str)\n-\t\treturn; /* no chance to report error */\n+\tif (!str) {\n+\t\twarning(\"malloc failed: '%s'\", strerror(errno));\n+\t\treturn;\n+\t}\n\n \tva_start(ap, fmt);\n \tvsnprintf(str, str_len + 1, fmt, ap);\n@@ -42,8 +44,10 @@ void syslog(int priority, const char *fmt, ...)\n\n \twhile ((pos = strstr(str, \"%1\")) != NULL) {\n \t\tstr = realloc(str, ++str_len + 1);\n-\t\tif (!str)\n+\t\tif (!str) {\n+\t\t\twarning(\"realloc failed: '%s'\", strerror(errno));\n \t\t\treturn;\n+\t\t}\n \t\tmemmove(pos + 2, pos + 1, strlen(pos));\n \t\tpos[1] = ' ';\n \t}\n"}]}