Re: [PATCH 1/1] libtracecmd: fix memory leak on partial reverse iteration

On Mon, 2025-11-24 at 11:58 -0500, Steven Rostedt wrote:
> On Mon, 24 Nov 2025 09:14:06 +0000
> "MOESSBAUER, Felix" <[email protected]> wrote:
> 
> > Hi,
> > 
> > while thinking about it again, I noticed that the callback based
> > iteration interface relies on some assumptions which are not
> > documented. While probably obvious, it should be documented what is NOT
> > allowed to be done inside the callback (i.e. calling any 
> > tracecmd_iterate_* function, but also calling tracecmd_read_* and
> > alike?).
> 
> Agreed that this should be documented. I wouldn't say it is "NOT" allowed,
> but instead specify what the side effects are.

Agree, makes sense.

> 
> I think stating how the iterator is somewhat implemented is good, as it
> should allow the user to assume its behavior.
> 
> Basically saying that the internal cursor of the iterator is:
> 
>   do {
> 	record = tracecmd_read_next_data(handle, &cpu);
> 	if (!record)
> 		break;
> 	ret = callback(handle, record, cpu, data);
>   } while (!ret);
> 
> And state that any reads in the callback can change the iterator, and cause
> it to behave differently, just like it would in the above code.

I'm just wondering if all possible / reasonable user interactions lead
to a state that does not leak memory. Anyways, just documenting it is
IMHO sufficient.

> 
> Hmm, my fix of the reverse code had the last record get read again. That's
> different than the iterator itself. I should get rid of the "- 4" and make
> the continue read the next record after the one last read too.

Yep!

Felix

> 
> Thanks,
> 
> -- Steve

-- 
Siemens AG
Linux Expert Center
Friedrich-Ludwig-Bauer-Str. 3
85748 Garching, Germany