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

[Lluís Vilanova]

Alexey Kardashevskiy writes:

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

Yes, that's right. All I'm saying is that the commit message is a little bit
misleading. These are not the line numbers of a trace, but the line numbers of a
list of events to enable when QEMU starts.


Thanks,
  Lluis


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

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