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

Simon Glass sjg at chromium.org
Mon May 28 01:45:08 UTC 2018


Hi Ramon,

On 27 May 2018 at 02:35, Ramon Fried <ramon.fried at gmail.com> wrote:
> On Sun, May 27, 2018 at 1:18 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Ramon,
>>
>> On 26 May 2018 at 00:05, Ramon Fried <ramon.fried at gmail.com> wrote:
>>> 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 ?
>>
>> The problem is that people want to be able to resize the buffer so
>> that they can try again, so they need to know the correct size.
>>
>> Can you make it report that it overflowed, and print the correct size?
> The correct size can only be printed in the end of the tracing.
> Maybe we can use WARN_ONCE when it first occurs and later when the user
> type "iotrace stats" it will state the required buffer size to
> accommodate all of the
> entries.
> What do you think ?

Sounds good to me.

Regards,
Simon


More information about the U-Boot mailing list