Re: [ECOS] problem with print_trace_buffer

[Jonathan Larmour <jlarmour at redhat dot co dot uk>]

Chris Morrow wrote:
> 
> I believe there is a problem with print_trace_buffer() in
> .../infra/current/buffer.cxx

Yep, good catch.
 
> If eCos is configured to dump the trace buffer everytime the
> buffer is full, the loop in print_trace_buffer never terminates.
> 
> Changing the code from
> 
> i = start;
> do {
>         ... print a bunch of stuff ...
>         i++;
>         if ( i == CYGDBG_INFRA_DEBUG_TRACE_BUFFER_SIZE)
>                 i = 0;
> } while (i != end)
> 
> to
> 
> i = start;
> do {
>         if ( i == CYGDBG_INFRA_DEBUG_TRACE_BUFFER_SIZE)
>                 i = 0;
>         ... print a bunch of stuff ...
>         i++;
> } while (i != end)
> 
> fixes the problem.

I don't think this is quite right. If you _are_ wrapping around, and you
had traced exactly CYGDBG_INFRA_DEBUG_TRACE_BUFFER_SIZE records (i.e.
end==0), then you'd get stuck in a loop then instead.

What you actually want would be more like:

i = start;
if ( i == CYGDBG_INFRA_DEBUG_TRACE_BUFFER_SIZE)
        i = 0;
do {
        ... print a bunch of stuff ...
        i++;
        if ( i == CYGDBG_INFRA_DEBUG_TRACE_BUFFER_SIZE)
                i = 0;
} while (i != end)

But what I'll check in is instead to change increment_buffer_pos() for the
case when printing when full, from:

#elif defined(CYGDBG_INFRA_DEBUG_TRACE_BUFFER_PRINT)
        print_trace_buffer();
        cyg_infra_trace_buffer_pos = 0;
        cyg_infra_trace_buffer_wrap = false;

to just:

#elif defined(CYGDBG_INFRA_DEBUG_TRACE_BUFFER_PRINT)
        cyg_infra_trace_buffer_pos = 0;
        print_trace_buffer();

Okay?

I've noticed that this code has another bug - an empty trace buffer will
actually cause the entire contents to be printed out. Quite confusing if
you've just called cyg_trace_print() to print it out and "empty" it. I'll
make a note of that internally.

Jifl
-- 
Red Hat, 35 Cambridge Place, Cambridge, UK. CB2 1NS  Tel: +44 (1223) 728762
"Plan to be spontaneous tomorrow."  ||  These opinions are all my own fault