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

[Alexey Kardashevskiy]

On 05/29/2014 10:42 PM, Lluís Vilanova wrote:
> 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.


Sorry, I am not following you. It shows me this:

qemu-system-ppc64:/home/aik/qemu_trace_events:75: WARNING: trace event
'mememe' does not exist

and continues. This exactly what I wanted.


> 
> 
> 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
> 
> 


-- 
Alexey