{"thread":{"id":"24078","subject":"[PATCH next] fast-import: revert die_nicely() to vsnprintf","startedAt":"2010-06-10T19:55:30Z","lastAt":"2010-06-11T15:02:50Z","messageCount":3,"participants":["Thomas Rast","Erik Faye-Lund"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"143475","messageId":"29df1a625a82bd2d3fcee92068b3a15f56435c66.1276199549.git.trast@student.ethz.ch","threadId":"24078","inReplyTo":null,"subject":"[PATCH next] fast-import: revert die_nicely() to vsnprintf","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-06-10T19:55:30Z","receivedAt":"2010-06-10T19:55:30Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"ebaa79f (Make report() from usage.c public as vreportf() and use it.,\n2010-03-06) changed fast-import's die_nicely() to use vreportf().\n\nThis requires some more care though. First it forgot that we also\nneed to reformat the message for the crash report. Second, vreportf()\nuses vsnprintf(), which does not call va_end(). This leaves the\nva_list passed to it in an undefined state. Therefore we need to make\na copy of this va_list so that we can reuse it.\n\nSigned-off-by: Thomas Rast \n---\n\nAnother valgrind catch. I think that'll be the last one for today.\nThanks for your attention.\n\n\n fast-import.c | 4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex c0728c2..1fa5de4 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -483,12 +483,14 @@ static void dump_marks(void);\n static NORETURN void die_nicely(const char *err, va_list params)\n {\n \tstatic int zombie;\n+\tva_list saved_params;\n+\tva_copy(saved_params, params);\n \n \tvreportf(\"fatal: \", err, params);\n \n \tif (!zombie) {\n \t\tchar message[2 * PATH_MAX];\n-\n+\t\tvsnprintf(message, sizeof(message), err, saved_params);\n \t\tzombie = 1;\n \t\twrite_crash_report(message);\n \t\tend_packfile();\n-- \n1.7.1.561.g94582\n"},{"id":"143505","messageId":"AANLkTikIQeY1ZieEkev_zjDSVI9W1NpfhiyTNCaToeys@mail.gmail.com","threadId":"24078","inReplyTo":"29df1a625a82bd2d3fcee92068b3a15f56435c66.1276199549.git.trast@student.ethz.ch","subject":"Re: [PATCH next] fast-import: revert die_nicely() to vsnprintf","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2010-06-11T13:18:10Z","receivedAt":"2010-06-11T13:18:10Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Jun 10, 2010 at 9:55 PM, Thomas Rast wrote:\n> ebaa79f (Make report() from usage.c public as vreportf() and use it.,\n> 2010-03-06) changed fast-import's die_nicely() to use vreportf().\n>\n> This requires some more care though.  First it forgot that we also\n> need to reformat the message for the crash report.  Second, vreportf()\n> uses vsnprintf(), which does not call va_end().  This leaves the\n> va_list passed to it in an undefined state.  Therefore we need to make\n> a copy of this va_list so that we can reuse it.\n>\n> Signed-off-by: Thomas Rast \n> ---\n>\n> Another valgrind catch.  I think that'll be the last one for today.\n> Thanks for your attention.\n>\n>\n>  fast-import.c |    4 +++-\n>  1 files changed, 3 insertions(+), 1 deletions(-)\n>\n> diff --git a/fast-import.c b/fast-import.c\n> index c0728c2..1fa5de4 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -483,12 +483,14 @@ static void dump_marks(void);\n>  static NORETURN void die_nicely(const char *err, va_list params)\n>  {\n>        static int zombie;\n> +       va_list saved_params;\n> +       va_copy(saved_params, params);\n\nUgh. We don't use the va_copy for portability reasons; it's C99, and\nimpossible to implement in a portable way on non-C99 systems.\n\n-- \nErik \"kusma\" Faye-Lund\n"},{"id":"143513","messageId":"769689aa933aa5b0d207e94983328c03db105594.1276268216.git.trast@student.ethz.ch","threadId":"24078","inReplyTo":"AANLkTikIQeY1ZieEkev_zjDSVI9W1NpfhiyTNCaToeys@mail.gmail.com","subject":"[PATCH v2] fast-import: die_nicely() back to vsnprintf (reverts part of ebaa79f)","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2010-06-11T15:02:50Z","receivedAt":"2010-06-11T15:02:50Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"ebaa79f (Make report() from usage.c public as vreportf() and use it.,\n2010-03-06) changed fast-import's die_nicely() to use vreportf().\nUnfortunately this is not possible: we need the message again for\nwrite_report(), and vreportf() uses vsnprintf(), which invalidates the\nva_list. As pointed out by Erik Faye-Lund, va_copy is C99 and thus\nnot an option.\n\nSo revert the part of ebaa79f that pertains to die_nicely().\n\nSigned-off-by: Thomas Rast \n---\n\nErik Faye-Lund wrote:\n> Ugh. We don't use the va_copy for portability reasons; it's C99, and\n> impossible to implement in a portable way on non-C99 systems.\n\nAh well, that makes the patch easier to write though more annoying :-)\n\n fast-import.c | 8 +++++---\n 1 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex c0728c2..1e5d66e 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -483,12 +483,14 @@ static void dump_marks(void);\n static NORETURN void die_nicely(const char *err, va_list params)\n {\n \tstatic int zombie;\n+\tchar message[2 * PATH_MAX];\n \n-\tvreportf(\"fatal: \", err, params);\n+\tvsnprintf(message, sizeof(message), err, params);\n+\tfputs(\"fatal: \", stderr);\n+\tfputs(message, stderr);\n+\tfputc('\\n', stderr);\n \n \tif (!zombie) {\n-\t\tchar message[2 * PATH_MAX];\n-\n \t\tzombie = 1;\n \t\twrite_crash_report(message);\n \t\tend_packfile();\n-- \n1.7.1.561.g94582\n"}]}