[PATCH v2 0/2] tracetool: remove no_check_foo() and if(true){..} blocks

[PATCH v2 0/2] tracetool: remove no_check_foo() and if(true){..} blocks

[Tanish Desai]

This patch series eliminates unnecessary
if (true) { no_check_foo(...) } blocks and
integrates the no_check_foo(...) logic directly
into trace_foo(...). This results in cleaner,
more maintainable code generation.

A new backend attribute, TRACE_EVENT_GET_STATE,
is introduced. When enabled, it automatically
generates conditional block :
if (trace_event_get_state(...)) { ... }. The
generate() function emits code within this
conditional structure for that backend.

Previously, without TRACE_EVENT_GET_STATE,
each backend was required to manually implement
out("if (trace_event_get_state(...)) {") in its
generate() function, leading to code duplication.

Tanish Desai (2):
  tracetool: add CHECK_TRACE_EVENT_GET_STATE
  tracetool/format: remove redundent trace-event checks

 scripts/tracetool/__init__.py         |  1 -
 scripts/tracetool/backend/__init__.py | 26 ++++++++++++++++-------
 scripts/tracetool/backend/ftrace.py   | 12 +++++------
 scripts/tracetool/backend/log.py      | 10 ++++-----
 scripts/tracetool/backend/simple.py   |  9 ++------
 scripts/tracetool/backend/syslog.py   |  8 ++-----
 scripts/tracetool/format/h.py         | 30 ++++++++++-----------------
 7 files changed, 43 insertions(+), 53 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] tracetool: add CHECK_TRACE_EVENT_GET_STATE

[Tanish Desai]

New attributed added in backends
CHECK_TRACE_EVENT_GET_STATE which when
present and is True wraps the code generated
by generate function in check_trace_event_get_state
check else it is outside the conditional block.

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
---
 scripts/tracetool/__init__.py         |  1 -
 scripts/tracetool/backend/__init__.py | 26 ++++++++++++++++-------
 scripts/tracetool/format/h.py         | 30 ++++++++++-----------------
 3 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 2ae2e562d6..d0a02c45d7 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -332,7 +332,6 @@ def formats(self):
         return self._FMT.findall(self.fmt)
 
     QEMU_TRACE               = "trace_%(name)s"
-    QEMU_TRACE_NOCHECK       = "_nocheck__" + QEMU_TRACE
     QEMU_TRACE_TCG           = QEMU_TRACE + "_tcg"
     QEMU_DSTATE              = "_TRACE_%(NAME)s_DSTATE"
     QEMU_BACKEND_DSTATE      = "TRACE_%(NAME)s_BACKEND_DSTATE"
diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
index 7bfcc86cc5..4194719e52 100644
--- a/scripts/tracetool/backend/__init__.py
+++ b/scripts/tracetool/backend/__init__.py
@@ -23,6 +23,8 @@
 Attribute Description
 ========= ====================================================================
 PUBLIC    If exists and is set to 'True', the backend is considered "public".
+CHECK_TRACE_EVENT_GET_STATE    If exists and is set to 'True', generate function
+emits code inside check_trace_event_get_state check.
 ========= ====================================================================
 
 
@@ -101,22 +103,32 @@ class Wrapper:
     def __init__(self, backends, format):
         self._backends = [backend.replace("-", "_") for backend in backends]
         self._format = format.replace("-", "_")
+        self.check_trace_event_get_state = False
         for backend in self._backends:
             assert exists(backend)
         assert tracetool.format.exists(self._format)
+        for backend in self.backend_modules():
+            check_trace_event_get_state = getattr(backend, "CHECK_TRACE_EVENT_GET_STATE", False)
+            self.check_trace_event_get_state = self.check_trace_event_get_state or check_trace_event_get_state
 
-    def _run_function(self, name, *args, **kwargs):
+    def backend_modules(self):
         for backend in self._backends:
-            func = tracetool.try_import("tracetool.backend." + backend,
-                                        name % self._format, None)[1]
-            if func is not None:
-                func(*args, **kwargs)
+             module = tracetool.try_import("tracetool.backend." + backend)[1]
+             if module is not None:
+                 yield module
+
+    def _run_function(self, name, *args, check_trace_event_get_state=None, **kwargs):
+        for backend in self.backend_modules():
+            func = getattr(backend,name%self._format,None)
+            if func is not None and (check_trace_event_get_state is None or
+                    check_trace_event_get_state == getattr(backend, 'CHECK_TRACE_EVENT_GET_STATE', False)):
+                    func(*args, **kwargs)
 
     def generate_begin(self, events, group):
         self._run_function("generate_%s_begin", events, group)
 
-    def generate(self, event, group):
-        self._run_function("generate_%s", event, group)
+    def generate(self, event, group, check_trace_event_get_state=None):
+        self._run_function("generate_%s", event, group, check_trace_event_get_state=check_trace_event_get_state)
 
     def generate_backend_dstate(self, event, group):
         self._run_function("generate_%s_backend_dstate", event, group)
diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
index ea126b07ea..0ceb49eef5 100644
--- a/scripts/tracetool/format/h.py
+++ b/scripts/tracetool/format/h.py
@@ -59,33 +59,25 @@ def generate(events, backend, group):
 
         out('    false)')
 
-        # tracer without checks
         out('',
             'static inline void %(api)s(%(args)s)',
             '{',
-            api=e.api(e.QEMU_TRACE_NOCHECK),
+            api=e.api(),
             args=e.args)
 
         if "disable" not in e.properties:
-            backend.generate(e, group)
-
+            backend.generate(e, group, check_trace_event_get_state=False)
+
+        if backend.check_trace_event_get_state:
+            if "disable" not in e.properties:
+                event_id = 'TRACE_' + e.name.upper()
+                cond = "trace_event_get_state(%s)" % event_id
+                out('    if (%(cond)s) {',
+                        cond=cond)
+                backend.generate(e, group, check_trace_event_get_state=True)
+                out('    }')
         out('}')
 
-        cond = "true"
-
-        out('',
-            'static inline void %(api)s(%(args)s)',
-            '{',
-            '    if (%(cond)s) {',
-            '        %(api_nocheck)s(%(names)s);',
-            '    }',
-            '}',
-            api=e.api(),
-            api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
-            args=e.args,
-            names=", ".join(e.args.names()),
-            cond=cond)
-
     backend.generate_end(events, group)
 
     out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
-- 
2.34.1

Re: [PATCH v2 1/2] tracetool: add CHECK_TRACE_EVENT_GET_STATE

[Daniel P. Berrangé]

On Wed, Aug 06, 2025 at 03:05:38PM +0000, Tanish Desai wrote:
> New attributed added in backends
> CHECK_TRACE_EVENT_GET_STATE which when
> present and is True wraps the code generated
> by generate function in check_trace_event_get_state
> check else it is outside the conditional block.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> ---
>  scripts/tracetool/__init__.py         |  1 -
>  scripts/tracetool/backend/__init__.py | 26 ++++++++++++++++-------
>  scripts/tracetool/format/h.py         | 30 ++++++++++-----------------
>  3 files changed, 30 insertions(+), 27 deletions(-)
> 
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 2ae2e562d6..d0a02c45d7 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -332,7 +332,6 @@ def formats(self):
>          return self._FMT.findall(self.fmt)
>  
>      QEMU_TRACE               = "trace_%(name)s"
> -    QEMU_TRACE_NOCHECK       = "_nocheck__" + QEMU_TRACE

This is just removing obsolete code

>      QEMU_TRACE_TCG           = QEMU_TRACE + "_tcg"
>      QEMU_DSTATE              = "_TRACE_%(NAME)s_DSTATE"
>      QEMU_BACKEND_DSTATE      = "TRACE_%(NAME)s_BACKEND_DSTATE"

> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index ea126b07ea..0ceb49eef5 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -59,33 +59,25 @@ def generate(events, backend, group):
>  
>          out('    false)')
>  
> -        # tracer without checks
>          out('',
>              'static inline void %(api)s(%(args)s)',
>              '{',
> -            api=e.api(e.QEMU_TRACE_NOCHECK),
> +            api=e.api(),
>              args=e.args)

This bit is removing more obsolete code

>  
>          if "disable" not in e.properties:
> -            backend.generate(e, group)
> -
> +            backend.generate(e, group, check_trace_event_get_state=False)
> +
> +        if backend.check_trace_event_get_state:
> +            if "disable" not in e.properties:
> +                event_id = 'TRACE_' + e.name.upper()
> +                cond = "trace_event_get_state(%s)" % event_id
> +                out('    if (%(cond)s) {',
> +                        cond=cond)
> +                backend.generate(e, group, check_trace_event_get_state=True)
> +                out('    }')
>          out('}')

This is the actual new functionality

>  
> -        cond = "true"
> -
> -        out('',
> -            'static inline void %(api)s(%(args)s)',
> -            '{',
> -            '    if (%(cond)s) {',
> -            '        %(api_nocheck)s(%(names)s);',
> -            '    }',
> -            '}',
> -            api=e.api(),
> -            api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
> -            args=e.args,
> -            names=", ".join(e.args.names()),
> -            cond=cond)
> -

This is further obsolete code.

It is best to have new functionality added in a separate commit
from the removal of obsolete code.

I've co-incidentally got removal of this obsolete code in the
tracing test suite series I posted, so one will need to be
rebased on top of the other, depending on what order Stefan
wants to take the patches.

>      backend.generate_end(events, group)
>  
>      out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
> -- 
> 2.34.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 1/2] tracetool: add CHECK_TRACE_EVENT_GET_STATE

[Paolo Bonzini]

On 8/6/25 21:13, Daniel P. Berrangé wrote:
> It is best to have new functionality added in a separate commit
> from the removal of obsolete code.

My mistake - when Tanish and I "un-rebased" these patches from on top of 
yours, we placed the removal of the "nocheck" functions in patch 1 
instead of patch 2 where it made more sense.

> I've co-incidentally got removal of this obsolete code in the
> tracing test suite series I posted, so one will need to be
> rebased on top of the other, depending on what order Stefan
> wants to take the patches.

If the main blocker (the /dev/stdout issue) is fixed quickly, your 
patches should go in first because they aid in reviewing this one.

Tanish has started school again, so he'll probably concentrate on 
finishing what he has of the Rust work and I'll resubmit his patches later.

Paolo

Re: [PATCH v2 1/2] tracetool: add CHECK_TRACE_EVENT_GET_STATE

[Daniel P. Berrangé]

On Thu, Aug 07, 2025 at 12:35:39PM +0200, Paolo Bonzini wrote:
> On 8/6/25 21:13, Daniel P. Berrangé wrote:
> > It is best to have new functionality added in a separate commit
> > from the removal of obsolete code.
> 
> My mistake - when Tanish and I "un-rebased" these patches from on top of
> yours, we placed the removal of the "nocheck" functions in patch 1 instead
> of patch 2 where it made more sense.
> 
> > I've co-incidentally got removal of this obsolete code in the
> > tracing test suite series I posted, so one will need to be
> > rebased on top of the other, depending on what order Stefan
> > wants to take the patches.
> 
> If the main blocker (the /dev/stdout issue) is fixed quickly, your patches
> should go in first because they aid in reviewing this one.

I posted a new version that drops the stdout patch

  https://lists.nongnu.org/archive/html/qemu-devel/2025-08/msg02716.html

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 1/2] tracetool: add CHECK_TRACE_EVENT_GET_STATE

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 6664 bytes --]

On Wed, Aug 06, 2025 at 03:05:38PM +0000, Tanish Desai wrote:
> New attributed added in backends
> CHECK_TRACE_EVENT_GET_STATE which when
> present and is True wraps the code generated
> by generate function in check_trace_event_get_state
> check else it is outside the conditional block.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> ---
>  scripts/tracetool/__init__.py         |  1 -
>  scripts/tracetool/backend/__init__.py | 26 ++++++++++++++++-------
>  scripts/tracetool/format/h.py         | 30 ++++++++++-----------------
>  3 files changed, 30 insertions(+), 27 deletions(-)
> 
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 2ae2e562d6..d0a02c45d7 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -332,7 +332,6 @@ def formats(self):
>          return self._FMT.findall(self.fmt)
>  
>      QEMU_TRACE               = "trace_%(name)s"
> -    QEMU_TRACE_NOCHECK       = "_nocheck__" + QEMU_TRACE
>      QEMU_TRACE_TCG           = QEMU_TRACE + "_tcg"
>      QEMU_DSTATE              = "_TRACE_%(NAME)s_DSTATE"
>      QEMU_BACKEND_DSTATE      = "TRACE_%(NAME)s_BACKEND_DSTATE"
> diff --git a/scripts/tracetool/backend/__init__.py b/scripts/tracetool/backend/__init__.py
> index 7bfcc86cc5..4194719e52 100644
> --- a/scripts/tracetool/backend/__init__.py
> +++ b/scripts/tracetool/backend/__init__.py
> @@ -23,6 +23,8 @@
>  Attribute Description
>  ========= ====================================================================
>  PUBLIC    If exists and is set to 'True', the backend is considered "public".
> +CHECK_TRACE_EVENT_GET_STATE    If exists and is set to 'True', generate function
> +emits code inside check_trace_event_get_state check.
>  ========= ====================================================================

The '=' table formatting is broken. Further down in the file there is an
example of a wider first column and how the second column cells wrap:

=============================== ==============================================
Function                        Description
=============================== ==============================================
generate_<format>_begin(events) Generate backend- and format-specific file
                                header contents.

Please follow that style for consistency. You could also check reST
markup syntax alternatives like an unordered list if tables are too
clumsy.

>  
>  
> @@ -101,22 +103,32 @@ class Wrapper:
>      def __init__(self, backends, format):
>          self._backends = [backend.replace("-", "_") for backend in backends]
>          self._format = format.replace("-", "_")
> +        self.check_trace_event_get_state = False
>          for backend in self._backends:
>              assert exists(backend)
>          assert tracetool.format.exists(self._format)
> +        for backend in self.backend_modules():
> +            check_trace_event_get_state = getattr(backend, "CHECK_TRACE_EVENT_GET_STATE", False)
> +            self.check_trace_event_get_state = self.check_trace_event_get_state or check_trace_event_get_state
>  
> -    def _run_function(self, name, *args, **kwargs):
> +    def backend_modules(self):
>          for backend in self._backends:
> -            func = tracetool.try_import("tracetool.backend." + backend,
> -                                        name % self._format, None)[1]
> -            if func is not None:
> -                func(*args, **kwargs)
> +             module = tracetool.try_import("tracetool.backend." + backend)[1]
> +             if module is not None:
> +                 yield module
> +
> +    def _run_function(self, name, *args, check_trace_event_get_state=None, **kwargs):
> +        for backend in self.backend_modules():
> +            func = getattr(backend,name%self._format,None)

Spaces are missing on this line: getattr(backend, name % self._format, None)

> +            if func is not None and (check_trace_event_get_state is None or
> +                    check_trace_event_get_state == getattr(backend, 'CHECK_TRACE_EVENT_GET_STATE', False)):
> +                    func(*args, **kwargs)
>  
>      def generate_begin(self, events, group):
>          self._run_function("generate_%s_begin", events, group)
>  
> -    def generate(self, event, group):
> -        self._run_function("generate_%s", event, group)
> +    def generate(self, event, group, check_trace_event_get_state=None):
> +        self._run_function("generate_%s", event, group, check_trace_event_get_state=check_trace_event_get_state)
>  
>      def generate_backend_dstate(self, event, group):
>          self._run_function("generate_%s_backend_dstate", event, group)
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index ea126b07ea..0ceb49eef5 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -59,33 +59,25 @@ def generate(events, backend, group):
>  
>          out('    false)')
>  
> -        # tracer without checks
>          out('',
>              'static inline void %(api)s(%(args)s)',
>              '{',
> -            api=e.api(e.QEMU_TRACE_NOCHECK),
> +            api=e.api(),
>              args=e.args)
>  
>          if "disable" not in e.properties:
> -            backend.generate(e, group)
> -
> +            backend.generate(e, group, check_trace_event_get_state=False)
> +
> +        if backend.check_trace_event_get_state:

This line can be indented...

> +            if "disable" not in e.properties:

...and this duplicate "disable" check can be dropped.

> +                event_id = 'TRACE_' + e.name.upper()
> +                cond = "trace_event_get_state(%s)" % event_id
> +                out('    if (%(cond)s) {',
> +                        cond=cond)
> +                backend.generate(e, group, check_trace_event_get_state=True)
> +                out('    }')
>          out('}')
>  
> -        cond = "true"
> -
> -        out('',
> -            'static inline void %(api)s(%(args)s)',
> -            '{',
> -            '    if (%(cond)s) {',
> -            '        %(api_nocheck)s(%(names)s);',
> -            '    }',
> -            '}',
> -            api=e.api(),
> -            api_nocheck=e.api(e.QEMU_TRACE_NOCHECK),
> -            args=e.args,
> -            names=", ".join(e.args.names()),
> -            cond=cond)
> -
>      backend.generate_end(events, group)
>  
>      out('#endif /* TRACE_%s_GENERATED_TRACERS_H */' % group.upper())
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2 2/2] tracetool/format: remove redundent trace-event checks

[Tanish Desai]

Remove redundent if(check_trace_event) check
from individual backends.
Adding CHECK_TRACE_EVENT_GET_STATE in log,syslog,
dtrace and simple backend.

Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
---
 scripts/tracetool/backend/ftrace.py | 12 +++++-------
 scripts/tracetool/backend/log.py    | 10 ++++------
 scripts/tracetool/backend/simple.py |  9 ++-------
 scripts/tracetool/backend/syslog.py |  8 ++------
 4 files changed, 13 insertions(+), 26 deletions(-)

diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index 5fa30ccc08..c65d3b89b3 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -16,6 +16,7 @@
 
 
 PUBLIC = True
+CHECK_TRACE_EVENT_GET_STATE = True
 
 
 def generate_h_begin(events, group):
@@ -28,11 +29,10 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    out('    {',
-        '        char ftrace_buf[MAX_TRACE_STRLEN];',
-        '        int unused __attribute__ ((unused));',
-        '        int trlen;',
-        '        if (trace_event_get_state(%(event_id)s)) {',
+    out('        {',
+        '            char ftrace_buf[MAX_TRACE_STRLEN];',
+        '            int unused __attribute__ ((unused));',
+        '            int trlen;',
         '#line %(event_lineno)d "%(event_filename)s"',
         '            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
         '                             "%(name)s " %(fmt)s "\\n" %(argnames)s);',
@@ -40,10 +40,8 @@ def generate_h(event, group):
         '            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
         '            unused = write(trace_marker_fd, ftrace_buf, trlen);',
         '        }',
-        '    }',
         name=event.name,
         args=event.args,
-        event_id="TRACE_" + event.name.upper(),
         event_lineno=event.lineno,
         event_filename=event.filename,
         fmt=event.fmt.rstrip("\n"),
diff --git a/scripts/tracetool/backend/log.py b/scripts/tracetool/backend/log.py
index eb50ceea34..2aa180f4b4 100644
--- a/scripts/tracetool/backend/log.py
+++ b/scripts/tracetool/backend/log.py
@@ -16,6 +16,7 @@
 
 
 PUBLIC = True
+CHECK_TRACE_EVENT_GET_STATE = True
 
 
 def generate_h_begin(events, group):
@@ -28,14 +29,11 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
-
-    out('    if (%(cond)s && qemu_loglevel_mask(LOG_TRACE)) {',
+    out('        if (qemu_loglevel_mask(LOG_TRACE)) {',
         '#line %(event_lineno)d "%(event_filename)s"',
-        '        qemu_log("%(name)s " %(fmt)s "\\n"%(argnames)s);',
+        '            qemu_log("%(name)s " %(fmt)s "\\n"%(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
-        '    }',
-        cond=cond,
+        '        }',
         event_lineno=event.lineno,
         event_filename=event.filename,
         name=event.name,
diff --git a/scripts/tracetool/backend/simple.py b/scripts/tracetool/backend/simple.py
index 7c84c06b20..623ea7d8ed 100644
--- a/scripts/tracetool/backend/simple.py
+++ b/scripts/tracetool/backend/simple.py
@@ -16,7 +16,7 @@
 
 
 PUBLIC = True
-
+CHECK_TRACE_EVENT_GET_STATE = True
 
 def is_string(arg):
     strtype = ('const char*', 'char*', 'const char *', 'char *')
@@ -36,13 +36,8 @@ def generate_h_begin(events, group):
 
 
 def generate_h(event, group):
-    event_id = 'TRACE_' + event.name.upper()
-    cond = "trace_event_get_state(%s)" % event_id
-    out('    if (%(cond)s) {',
-        '        _simple_%(api)s(%(args)s);',
-        '    }',
+    out('        _simple_%(api)s(%(args)s);',
         api=event.api(),
-        cond=cond,
         args=", ".join(event.args.names()))
 
 
diff --git a/scripts/tracetool/backend/syslog.py b/scripts/tracetool/backend/syslog.py
index 3f82e54aab..04ec85717a 100644
--- a/scripts/tracetool/backend/syslog.py
+++ b/scripts/tracetool/backend/syslog.py
@@ -16,6 +16,7 @@
 
 
 PUBLIC = True
+CHECK_TRACE_EVENT_GET_STATE = True
 
 
 def generate_h_begin(events, group):
@@ -28,14 +29,9 @@ def generate_h(event, group):
     if len(event.args) > 0:
         argnames = ", " + argnames
 
-    cond = "trace_event_get_state(%s)" % ("TRACE_" + event.name.upper())
-
-    out('    if (%(cond)s) {',
-        '#line %(event_lineno)d "%(event_filename)s"',
+    out('#line %(event_lineno)d "%(event_filename)s"',
         '        syslog(LOG_INFO, "%(name)s " %(fmt)s %(argnames)s);',
         '#line %(out_next_lineno)d "%(out_filename)s"',
-        '    }',
-        cond=cond,
         event_lineno=event.lineno,
         event_filename=event.filename,
         name=event.name,
-- 
2.34.1

Re: [PATCH v2 2/2] tracetool/format: remove redundent trace-event checks

[Daniel P. Berrangé]

On Wed, Aug 06, 2025 at 03:05:39PM +0000, Tanish Desai wrote:
> Remove redundent if(check_trace_event) check
> from individual backends.
> Adding CHECK_TRACE_EVENT_GET_STATE in log,syslog,
> dtrace and simple backend.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> ---
>  scripts/tracetool/backend/ftrace.py | 12 +++++-------
>  scripts/tracetool/backend/log.py    | 10 ++++------
>  scripts/tracetool/backend/simple.py |  9 ++-------
>  scripts/tracetool/backend/syslog.py |  8 ++------
>  4 files changed, 13 insertions(+), 26 deletions(-)
> 
> diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
> index 5fa30ccc08..c65d3b89b3 100644
> --- a/scripts/tracetool/backend/ftrace.py
> +++ b/scripts/tracetool/backend/ftrace.py
> @@ -16,6 +16,7 @@
>  
>  
>  PUBLIC = True
> +CHECK_TRACE_EVENT_GET_STATE = True
>  
>  
>  def generate_h_begin(events, group):
> @@ -28,11 +29,10 @@ def generate_h(event, group):
>      if len(event.args) > 0:
>          argnames = ", " + argnames
>  
> -    out('    {',
> -        '        char ftrace_buf[MAX_TRACE_STRLEN];',
> -        '        int unused __attribute__ ((unused));',
> -        '        int trlen;',
> -        '        if (trace_event_get_state(%(event_id)s)) {',
> +    out('        {',
> +        '            char ftrace_buf[MAX_TRACE_STRLEN];',
> +        '            int unused __attribute__ ((unused));',
> +        '            int trlen;',

This results in unecessary nested code blocks & indent.
For example:

static inline void trace_test_blah(void *context, const char *filename)
{
    if (trace_event_get_state(TRACE_TEST_BLAH)) {
        {
            char ftrace_buf[MAX_TRACE_STRLEN];
            int unused __attribute__ ((unused));
            int trlen;
#line 4 "trace-events"
            trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
                             "test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
#line 33 "[stdout]"
            trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
            unused = write(trace_marker_fd, ftrace_buf, trlen);
        }
    }
}

Instead of


static inline void trace_test_blah(void *context, const char *filename)
{
    if (trace_event_get_state(TRACE_TEST_BLAH)) {
        char ftrace_buf[MAX_TRACE_STRLEN];
        int unused __attribute__ ((unused));
        int trlen;
#line 4 "trace-events"
        trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
                         "test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
#line 33 "[stdout]"
        trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
        unused = write(trace_marker_fd, ftrace_buf, trlen);
    }
}



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 2/2] tracetool/format: remove redundent trace-event checks

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

On Wed, Aug 06, 2025 at 03:05:39PM +0000, Tanish Desai wrote:
> Remove redundent if(check_trace_event) check
> from individual backends.
> Adding CHECK_TRACE_EVENT_GET_STATE in log,syslog,
> dtrace and simple backend.
> 
> Signed-off-by: Tanish Desai <tanishdesai37@gmail.com>
> ---
>  scripts/tracetool/backend/ftrace.py | 12 +++++-------
>  scripts/tracetool/backend/log.py    | 10 ++++------
>  scripts/tracetool/backend/simple.py |  9 ++-------
>  scripts/tracetool/backend/syslog.py |  8 ++------
>  4 files changed, 13 insertions(+), 26 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]