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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

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




> 
> 
> 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 <vilanova@ac.upc.edu>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@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