Re: [Qemu-devel] [PATCH] trace: Replace fprintf with error_report and print location

[Lluís Vilanova]

Alexey Kardashevskiy writes:

> This replaces fprintf(stderr) with error_report.
> This prints line number of the trace which does not exist or is not
> traceable.

A little nit pick; it shows an error when some of the events in the list of
events to enable (not the trace) does not exist or is not traceable.


Thanks,
  Lluis


> This moves local variables to the beginning of the function because of
> the QEMU coding style.

> Suggested-by: Lluís Vilanova <address@hidden>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---

> Lluís, or it is s/Suggested-by/From/ ?

> Stefan, this is made on top of "trace: Replace error with warning if event is 
> not defined"

> ---
>  trace/control.c | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)

> diff --git a/trace/control.c b/trace/control.c
> index 4aa02cf..4ee2bd2 100644
> --- a/trace/control.c
> +++ b/trace/control.c
> @@ -8,7 +8,7 @@
>   */
 
>  #include "trace/control.h"
> -
> +#include "qemu/error-report.h"
 
>  TraceEvent *trace_event_name(const char *name)
>  {
> @@ -81,18 +81,24 @@ TraceEvent *trace_event_pattern(const char *pat, 
> TraceEvent *ev)
 
>  void trace_backend_init_events(const char *fname)
>  {
> +    Location loc;
> +    FILE *fp;
> +    char line_buf[1024];
> +    size_t line_idx = 0;
> +
>      if (fname == NULL) {
>          return;
>      }
 
> -    FILE *fp = fopen(fname, "r");
> +    loc_push_none(&loc);
> +    loc_set_file(fname, 0);
> +    fp = fopen(fname, "r");
>      if (!fp) {
> -        fprintf(stderr, "error: could not open trace events file '%s': %s\n",
> -                fname, strerror(errno));
> +        error_report("%s", strerror(errno));
>          exit(1);
>      }
> -    char line_buf[1024];
>      while (fgets(line_buf, sizeof(line_buf), fp)) {
> +        loc_set_file(fname, ++line_idx);
>          size_t len = strlen(line_buf);
>          if (len > 1) {              /* skip empty lines */
>              line_buf[len - 1] = '\0';
> @@ -111,13 +117,11 @@ void trace_backend_init_events(const char *fname)
>              } else {
>                  TraceEvent *ev = trace_event_name(line_ptr);
>                  if (ev == NULL) {
> -                    fprintf(stderr,
> -                            "WARNING: trace event '%s' does not exist\n",
> -                            line_ptr);
> +                    error_report("WARNING: trace event '%s' does not exist",
> +                                 line_ptr);
>                  } else if (!trace_event_get_state_static(ev)) {
> -                    fprintf(stderr,
> -                            "WARNING: trace event '%s' is not traceable\n",
> -                            line_ptr);
> +                    error_report("WARNING: trace event '%s' is not 
> traceable\n",
> +                                 line_ptr);
>                  } else {
>                      trace_event_set_state_dynamic(ev, enable);
>                  }
> @@ -125,8 +129,9 @@ void trace_backend_init_events(const char *fname)
>          }
>      }
>      if (fclose(fp) != 0) {
> -        fprintf(stderr, "error: closing file '%s': %s\n",
> -                fname, strerror(errno));
> +        loc_set_file(fname, 0);
> +        error_report("%s", strerror(errno));
>          exit(1);
>      }
> +    loc_pop(&loc);
>  }
> -- 
> 1.8.4.rc4


-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth