[Qemu-devel] [PATCH] simpletrace: Move st_init() error reporting

[Qemu-devel] [PATCH] simpletrace: Move st_init() error reporting

[Stefan Hajnoczi]

User emulator builds do not have error_report() so it should not be used
by simpletrace.c.  In fact, error reporting inside simpletrace.c is
inappropriate and should be done by the caller instead.

This patch moves st_init() error reporting out to its caller,
vl.c:main().

Reported-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 simpletrace.c |    7 +++----
 simpletrace.h |    6 +++---
 vl.c          |    4 +++-
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/simpletrace.c b/simpletrace.c
index 9926ab3..f1dbb5e 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -14,7 +14,6 @@
 #include <time.h>
 #include <signal.h>
 #include <pthread.h>
-#include "qerror.h"
 #include "qemu-timer.h"
 #include "trace.h"
 
@@ -331,7 +330,7 @@ void st_flush_trace_buffer(void)
     flush_trace_file(true);
 }
 
-void st_init(const char *file)
+bool st_init(const char *file)
 {
     pthread_t thread;
     pthread_attr_t attr;
@@ -347,10 +346,10 @@ void st_init(const char *file)
     pthread_sigmask(SIG_SETMASK, &oldset, NULL);
 
     if (ret != 0) {
-        error_report("warning: unable to create trace file thread\n");
-        return;
+        return false;
     }
 
     atexit(st_flush_trace_buffer);
     st_set_trace_file(file);
+    return true;
 }
diff --git a/simpletrace.h b/simpletrace.h
index 3a5bd9f..8d893bd 100644
--- a/simpletrace.h
+++ b/simpletrace.h
@@ -37,11 +37,11 @@ void st_print_trace_file_status(FILE *stream, fprintf_function stream_printf);
 void st_set_trace_file_enabled(bool enable);
 bool st_set_trace_file(const char *file);
 void st_flush_trace_buffer(void);
-void st_init(const char *file);
+bool st_init(const char *file);
 #else
-static inline void st_init(const char *file)
+static inline bool st_init(const char *file)
 {
-    /* Do nothing */
+    return true;
 }
 #endif /* !CONFIG_SIMPLE_TRACE */
 
diff --git a/vl.c b/vl.c
index f500c4c..6e6a1ff 100644
--- a/vl.c
+++ b/vl.c
@@ -2748,7 +2748,9 @@ int main(int argc, char **argv, char **envp)
     }
     loc_set_none();
 
-    st_init(trace_file);
+    if (!st_init(trace_file)) {
+        fprintf(stderr, "warning: unable to initialize simple trace backend\n");
+    }
 
     /* If no data_dir is specified then try to find it relative to the
        executable path.  */
-- 
1.7.2.3

[Qemu-devel] Re: [PATCH] simpletrace: Move st_init() error reporting

[Blue Swirl]

Thanks, applied.

On Sun, Mar 13, 2011 at 10:14 PM, Stefan Hajnoczi
<stefanha@linux.vnet.ibm.com> wrote:
> User emulator builds do not have error_report() so it should not be used
> by simpletrace.c.  In fact, error reporting inside simpletrace.c is
> inappropriate and should be done by the caller instead.
>
> This patch moves st_init() error reporting out to its caller,
> vl.c:main().
>
> Reported-by: Blue Swirl <blauwirbel@gmail.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  simpletrace.c |    7 +++----
>  simpletrace.h |    6 +++---
>  vl.c          |    4 +++-
>  3 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/simpletrace.c b/simpletrace.c
> index 9926ab3..f1dbb5e 100644
> --- a/simpletrace.c
> +++ b/simpletrace.c
> @@ -14,7 +14,6 @@
>  #include <time.h>
>  #include <signal.h>
>  #include <pthread.h>
> -#include "qerror.h"
>  #include "qemu-timer.h"
>  #include "trace.h"
>
> @@ -331,7 +330,7 @@ void st_flush_trace_buffer(void)
>     flush_trace_file(true);
>  }
>
> -void st_init(const char *file)
> +bool st_init(const char *file)
>  {
>     pthread_t thread;
>     pthread_attr_t attr;
> @@ -347,10 +346,10 @@ void st_init(const char *file)
>     pthread_sigmask(SIG_SETMASK, &oldset, NULL);
>
>     if (ret != 0) {
> -        error_report("warning: unable to create trace file thread\n");
> -        return;
> +        return false;
>     }
>
>     atexit(st_flush_trace_buffer);
>     st_set_trace_file(file);
> +    return true;
>  }
> diff --git a/simpletrace.h b/simpletrace.h
> index 3a5bd9f..8d893bd 100644
> --- a/simpletrace.h
> +++ b/simpletrace.h
> @@ -37,11 +37,11 @@ void st_print_trace_file_status(FILE *stream, fprintf_function stream_printf);
>  void st_set_trace_file_enabled(bool enable);
>  bool st_set_trace_file(const char *file);
>  void st_flush_trace_buffer(void);
> -void st_init(const char *file);
> +bool st_init(const char *file);
>  #else
> -static inline void st_init(const char *file)
> +static inline bool st_init(const char *file)
>  {
> -    /* Do nothing */
> +    return true;
>  }
>  #endif /* !CONFIG_SIMPLE_TRACE */
>
> diff --git a/vl.c b/vl.c
> index f500c4c..6e6a1ff 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2748,7 +2748,9 @@ int main(int argc, char **argv, char **envp)
>     }
>     loc_set_none();
>
> -    st_init(trace_file);
> +    if (!st_init(trace_file)) {
> +        fprintf(stderr, "warning: unable to initialize simple trace backend\n");
> +    }
>
>     /* If no data_dir is specified then try to find it relative to the
>        executable path.  */
> --
> 1.7.2.3
>
>