From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ramon Fried Date: Sat, 26 May 2018 09:05:39 +0300 Subject: [U-Boot] [PATCH 6/6] common: iotrace: fix behaviour when buffer is full In-Reply-To: References: <20180525104128.7990-1-ramon.fried@gmail.com> <20180525104128.7990-7-ramon.fried@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Sat, May 26, 2018 at 5:07 AM, Simon Glass wrote: > Hi Ramon, > > On 25 May 2018 at 04:41, Ramon Fried 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 >> --- >> 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