The ltl2k class inherits from Monitor which requires subclasses to
implement fill_tracepoint_args_skel(). However, the ltl2k template
uses hardcoded tracepoint arguments rather than the placeholders that
this method would fill. The base class fill_trace_h() method calls
fill_tracepoint_args_skel() unconditionally, which was exposed when
the @not_implemented decorator was introduced.
Add a stub implementation that returns an empty string. Since the
ltl2k trace.h template does not contain the placeholder strings that
would be replaced, the empty return value has no effect on the
generated output while satisfying the base class interface contract.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/verification/rvgen/rvgen/ltl2k.py | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/verification/rvgen/rvgen/ltl2k.py b/tools/verification/rvgen/rvgen/ltl2k.py
index 94dc64af1716d..f1eafc16c754b 100644
--- a/tools/verification/rvgen/rvgen/ltl2k.py
+++ b/tools/verification/rvgen/rvgen/ltl2k.py
@@ -257,6 +257,9 @@ class ltl2k(generator.Monitor):
return '\n'.join(buf)
+ def fill_tracepoint_args_skel(self, tp_type) -> str:
+ return ""
+
def fill_monitor_class_type(self):
return "LTL_MON_EVENTS_ID"
--
2.52.0On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote: > The ltl2k class inherits from Monitor which requires subclasses to > implement fill_tracepoint_args_skel(). However, the ltl2k template > uses hardcoded tracepoint arguments rather than the placeholders that > this method would fill. The base class fill_trace_h() method calls > fill_tracepoint_args_skel() unconditionally, which was exposed when > the @not_implemented decorator was introduced. > > Add a stub implementation that returns an empty string. Since the > ltl2k trace.h template does not contain the placeholder strings that > would be replaced, the empty return value has no effect on the > generated output while satisfying the base class interface contract. > > Signed-off-by: Wander Lairson Costa <wander@redhat.com> 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? Thanks, Gabriele > --- > tools/verification/rvgen/rvgen/ltl2k.py | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tools/verification/rvgen/rvgen/ltl2k.py > b/tools/verification/rvgen/rvgen/ltl2k.py > index 94dc64af1716d..f1eafc16c754b 100644 > --- a/tools/verification/rvgen/rvgen/ltl2k.py > +++ b/tools/verification/rvgen/rvgen/ltl2k.py > @@ -257,6 +257,9 @@ class ltl2k(generator.Monitor): > > return '\n'.join(buf) > > + def fill_tracepoint_args_skel(self, tp_type) -> str: > + return "" > + > def fill_monitor_class_type(self): > return "LTL_MON_EVENTS_ID" >
On Wed, Jan 21, 2026 at 02:57:02PM +0100, Gabriele Monaco wrote: > On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote: > > The ltl2k class inherits from Monitor which requires subclasses to > > implement fill_tracepoint_args_skel(). However, the ltl2k template > > uses hardcoded tracepoint arguments rather than the placeholders that > > this method would fill. The base class fill_trace_h() method calls > > fill_tracepoint_args_skel() unconditionally, which was exposed when > > the @not_implemented decorator was introduced. > > > > Add a stub implementation that returns an empty string. Since the > > ltl2k trace.h template does not contain the placeholder strings that > > would be replaced, the empty return value has no effect on the > > generated output while satisfying the base class interface contract. > > > > Signed-off-by: Wander Lairson Costa <wander@redhat.com> > > 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. > Thanks, > Gabriele > > > --- > > tools/verification/rvgen/rvgen/ltl2k.py | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/tools/verification/rvgen/rvgen/ltl2k.py > > b/tools/verification/rvgen/rvgen/ltl2k.py > > index 94dc64af1716d..f1eafc16c754b 100644 > > --- a/tools/verification/rvgen/rvgen/ltl2k.py > > +++ b/tools/verification/rvgen/rvgen/ltl2k.py > > @@ -257,6 +257,9 @@ class ltl2k(generator.Monitor): > > > > return '\n'.join(buf) > > > > + def fill_tracepoint_args_skel(self, tp_type) -> str: > > + return "" > > + > > def fill_monitor_class_type(self): > > return "LTL_MON_EVENTS_ID" > > >
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: > > On Mon, 2026-01-19 at 17:45 -0300, Wander Lairson Costa wrote: > > > The ltl2k class inherits from Monitor which requires subclasses to > > > implement fill_tracepoint_args_skel(). However, the ltl2k template > > > uses hardcoded tracepoint arguments rather than the placeholders that > > > this method would fill. The base class fill_trace_h() method calls > > > fill_tracepoint_args_skel() unconditionally, which was exposed when > > > the @not_implemented decorator was introduced. > > > > > > Add a stub implementation that returns an empty string. Since the > > > ltl2k trace.h template does not contain the placeholder strings that > > > would be replaced, the empty return value has no effect on the > > > generated output while satisfying the base class interface contract. > > > > > > Signed-off-by: Wander Lairson Costa <wander@redhat.com> > > > > 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. > 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. > > Thanks, > > Gabriele > > > > > --- > > > tools/verification/rvgen/rvgen/ltl2k.py | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/tools/verification/rvgen/rvgen/ltl2k.py > > > b/tools/verification/rvgen/rvgen/ltl2k.py > > > index 94dc64af1716d..f1eafc16c754b 100644 > > > --- a/tools/verification/rvgen/rvgen/ltl2k.py > > > +++ b/tools/verification/rvgen/rvgen/ltl2k.py > > > @@ -257,6 +257,9 @@ class ltl2k(generator.Monitor): > > > > > > return '\n'.join(buf) > > > > > > + def fill_tracepoint_args_skel(self, tp_type) -> str: > > > + return "" > > > + > > > def fill_monitor_class_type(self): > > > return "LTL_MON_EVENTS_ID" > > > > > >
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. What do you think? Thanks, Gabriele
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 >
On Fri, 2026-01-23 at 09:19 -0300, Wander Lairson Costa wrote: > On Thu, Jan 22, 2026 at 02:49:59PM +0100, Gabriele Monaco wrote: > > 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"... Right, it ended up quite cryptic, I meant fill_tracepoint_args_skel() could stay in Monitor although not all Monitors need it, though I honestly prefer to move it away and not rely on the stub. > > 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. > But I get your stance and agree with that too, where fill_tracepoint_args_skel() goes is just nitpicking at this point. Thanks, Gabriele
On Fri, Jan 23, 2026 at 01:26:45PM +0100, Gabriele Monaco wrote: > On Fri, 2026-01-23 at 09:19 -0300, Wander Lairson Costa wrote: > > On Thu, Jan 22, 2026 at 02:49:59PM +0100, Gabriele Monaco wrote: > > > 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"... > > Right, it ended up quite cryptic, I meant fill_tracepoint_args_skel() could stay > in Monitor although not all Monitors need it, though I honestly prefer to move > it away and not rely on the stub. > > > > 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. > > > > But I get your stance and agree with that too, where fill_tracepoint_args_skel() > goes is just nitpicking at this point. > I will go with your early suggestion and drop all this related work in v2 and submitting a separate patch series addressing these interface issues. Python has some good tools [1,2] to handle that. I intend to make use of them. [1] https://docs.python.org/3/library/abc.html [2] https://typing.python.org/en/latest/spec/protocol.html > Thanks, > Gabriele >
© 2016 - 2026 Red Hat, Inc.