Re: [PATCH 18/26] rv/rvgen: add fill_tracepoint_args_skel stub to ltl2k

[Wander Lairson Costa <wander@redhat.com>]

On Thu, Jan 22, 2026 at 02:49:59PM +0100, Gabriele Monaco wrote:
> On Thu, 2026-01-22 at 10:10 -0300, Wander Lairson Costa wrote:
> > On Wed, Jan 21, 2026 at 02:53:03PM -0300, Wander Lairson Costa wrote:
> > > On Wed, Jan 21, 2026 at 02:57:02PM +0100, Gabriele Monaco wrote:
> > > > Mmh, this is a bit fishy though.
> > > > We the patch using the decorator seems fine, but highlights how this
> > > > method
> > > > isn't meant to be in Monitor if not all monitors use it..
> > > > Adding a stub here is just sweeping dust under the carpet.
> > > > 
> > > > Here should probably keep the common part of fill_trace_h() in Monitor
> > > > (e.g.
> > > > replacing MODEL_NAME and other common things) and create specific
> > > > implementations in dot2k and ltl2k for what is not common while calling
> > > > the
> > > > super() counterpart for the rest.
> > > > 
> > > > Does it make sense to you?
> > > 
> > > Yes, that is exactly my idea. Since the patch series were getting too
> > > long and my brain too rot, I thought would be better addressing this in
> > > a following up patch series. But I can work in the next version if you
> > > are not ok with that approach.
> 
> Good point, that can be a separate series so that we don't mix too many things,
> but I'd also separate the initial patch introducing the @not_implemented .
> 
> > I gave more thought on this matter yesterday before bed. Maybe this
> > isn't a issue on the design. Some methods on Monitor might just have a
> > harmless default behavior. I look into it more closely for next the
> > round.
> 
> Well, I believe that if a bunch of methods from the parent class don't need to
> be called and we have to create stubs just to avoid errors, those methods
> probably shouldn't be there in the first place.
> 
> That's particularly valid for the Container class, that won't ever need to fill
> tracepoints and other stuff.
> 
> Why fill_tracepoint_args_skel() is not required by LTL is an implementation
> detail, so that stub could even stay, in case future monitors are going to need
> the entire thing.
> Though I still find it cleaner to move that away too until there's a need for it
> shared in Monitor.
> 

I didn't catch what is included in "that"...

> What do you think?
> 

I agreed. fill_tracepoint_args_skel() makes sense in the Monitor class.
If a derived class doesn't need it, it is an implementation detail.

> Thanks,
> Gabriele
>