[U-Boot] [PATCH 6/6] common: iotrace: fix behaviour when buffer is full

[Ramon Fried]

On Sat, May 26, 2018 at 5:07 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Ramon,
>
> On 25 May 2018 at 04:41, Ramon Fried <ramon.fried at gmail.com> wrote:
>> When the buffer is full, there supposed to be no more
>> writes, the code however misses the else statement and
>> subsequently writes to arbitrary pointer location and increases
>> the offset.
>
> I don't think so. It writes to a local variable in this case. The
> point of this is to detect how much space would be needed to hold the
> I/O trace. Unless the pointer is incremented, there is no way to know.
You're right. I missed the initial assignment to rec.

>
> Perhaps instead, iotrace_get_buffer() should be updated to also return
> the number of valid records, as well as the pointer value?
>
It's a valid option, another one I have in mind is that
we can return immediately like I suggested but add one time warning
that states that the
buffer is full ?

>> This patch fixes that by returning immediately.
>>
>> Signed-off-by: Ramon Fried <ramon.fried at gmail.com>
>> ---
>>  common/iotrace.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/common/iotrace.c b/common/iotrace.c
>> index 74408a5dbb..5f06d2b250 100644
>> --- a/common/iotrace.c
>> +++ b/common/iotrace.c
>> @@ -55,6 +55,8 @@ static void add_record(int flags, const void *ptr, ulong value)
>>                 rec = (struct iotrace_record *)map_sysmem(
>>                                         iotrace.start + iotrace.offset,
>>                                         sizeof(value));
>> +       } else {
>> +               return;
>>         }
>>
>>         rec->timestamp = get_ticks();
>> --
>> 2.17.0
>>
>
> Regards,
> Simon