threads / patch / 24078

patchfast-import: revert die_nicely() to vsnprintf

Subject: [PATCH next] fast-import: revert die_nicely() to vsnprintf

## tl;dr

3 messages between Jun 10, 2010 and Jun 11, 2010. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Thomas Rast· Jun 10, 2010, 19:55 UTC · lore

ebaa79f (Make report() from usage.c public as vreportf() and use it., 2010-03-06) changed fast-import's die_nicely() to use vreportf().

This requires some more care though. First it forgot that we also need to reformat the message for the crash report. Second, vreportf() uses vsnprintf(), which does not call va_end(). This leaves the va_list passed to it in an undefined state. Therefore we need to make a copy of this va_list so that we can reuse it.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Another valgrind catch. I think that'll be the last one for today. Thanks for your attention.

 fast-import.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

Show changes to fast-import.c +3 −1

diff --git a/fast-import.c b/fast-import.c
index c0728c2..1fa5de4 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -483,12 +483,14 @@ static void dump_marks(void);
 static NORETURN void die_nicely(const char *err, va_list params)
 {
 	static int zombie;
+	va_list saved_params;
+	va_copy(saved_params, params);
 
 	vreportf("fatal: ", err, params);
 
 	if (!zombie) {
 		char message[2 * PATH_MAX];
-
+		vsnprintf(message, sizeof(message), err, saved_params);
 		zombie = 1;
 		write_crash_report(message);
 		end_packfile();

-- 
1.7.1.561.g94582

Erik Faye-Lund· Jun 11, 2010, 13:18 UTC · re: Thomas Rast · lore

Re: [PATCH next] fast-import: revert die_nicely() to vsnprintf

On Thu, Jun 10, 2010 at 9:55 PM, Thomas Rast <trast@student.ethz.ch> wrote:

Show 29 quoted lines

> ebaa79f (Make report() from usage.c public as vreportf() and use it.,
> 2010-03-06) changed fast-import's die_nicely() to use vreportf().
>
> This requires some more care though.  First it forgot that we also
> need to reformat the message for the crash report.  Second, vreportf()
> uses vsnprintf(), which does not call va_end().  This leaves the
> va_list passed to it in an undefined state.  Therefore we need to make
> a copy of this va_list so that we can reuse it.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
> Another valgrind catch.  I think that'll be the last one for today.
> Thanks for your attention.
>
>
>  fast-import.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/fast-import.c b/fast-import.c
> index c0728c2..1fa5de4 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -483,12 +483,14 @@ static void dump_marks(void);
>  static NORETURN void die_nicely(const char *err, va_list params)
>  {
>        static int zombie;
> +       va_list saved_params;
> +       va_copy(saved_params, params);

Ugh. We don't use the va_copy for portability reasons; it's C99, and impossible to implement in a portable way on non-C99 systems.

-- 
Erik "kusma" Faye-Lund

Thomas Rast· Jun 11, 2010, 15:02 UTC · re: Erik Faye-Lund · lore

[PATCH v2] fast-import: die_nicely() back to vsnprintf (reverts part of ebaa79f)

ebaa79f (Make report() from usage.c public as vreportf() and use it., 2010-03-06) changed fast-import's die_nicely() to use vreportf(). Unfortunately this is not possible: we need the message again for write_report(), and vreportf() uses vsnprintf(), which invalidates the va_list. As pointed out by Erik Faye-Lund, va_copy is C99 and thus not an option.

So revert the part of ebaa79f that pertains to die_nicely().

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Erik Faye-Lund wrote:

> Ugh. We don't use the va_copy for portability reasons; it's C99, and
> impossible to implement in a portable way on non-C99 systems.

Ah well, that makes the patch easier to write though more annoying :-)

 fast-import.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

Show changes to fast-import.c +5 −3

diff --git a/fast-import.c b/fast-import.c
index c0728c2..1e5d66e 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -483,12 +483,14 @@ static void dump_marks(void);
 static NORETURN void die_nicely(const char *err, va_list params)
 {
 	static int zombie;
+	char message[2 * PATH_MAX];
 
-	vreportf("fatal: ", err, params);
+	vsnprintf(message, sizeof(message), err, params);
+	fputs("fatal: ", stderr);
+	fputs(message, stderr);
+	fputc('\n', stderr);
 
 	if (!zombie) {
-		char message[2 * PATH_MAX];
-
 		zombie = 1;
 		write_crash_report(message);
 		end_packfile();

-- 
1.7.1.561.g94582