Alexey Kardashevskiy writes: > On 05/31/2014 12:08 AM, Lluís Vilanova wrote: >> 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.
> Like this? Thanks. > === > This replaces fprintf(stderr) with error_report. > This prints the line number of a list of events which does not exist or is > not traceable. > This moves local variables to the beginning of the function because of > the QEMU coding style. > === I think there is no need to describe what that function is doing. Instead I would say something like: Replaces fprintf(stdeer) with error_report. Also moves some local variables to the beginning of the function to comply with QEMU's coding style. Thanks, Lluis >> >> >> 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 <vilan...@ac.upc.edu> >>>>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru> >>>>> --- >>>> >>>>> 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); >>>>> } > -- > 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