This simplifies the Python code and reduces the size of the tracepoints.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tests/tracetool/ftrace.h | 28 ++++++----------------------
trace/ftrace.h | 1 +
trace/ftrace.c | 15 +++++++++++++++
scripts/tracetool/backend/ftrace.py | 12 ++----------
4 files changed, 24 insertions(+), 32 deletions(-)
diff --git a/tests/tracetool/ftrace.h b/tests/tracetool/ftrace.h
index fe22ea0f09f..1dfe4239413 100644
--- a/tests/tracetool/ftrace.h
+++ b/tests/tracetool/ftrace.h
@@ -21,18 +21,10 @@ extern uint16_t _TRACE_TEST_WIBBLE_DSTATE;
static inline void trace_test_blah(void *context, const char *filename)
{
- {
- char ftrace_buf[MAX_TRACE_STRLEN];
- int unused __attribute__ ((unused));
- int trlen;
- if (trace_event_get_state(TRACE_TEST_BLAH)) {
+ if (trace_event_get_state(TRACE_TEST_BLAH)) {
#line 4 "trace-events"
- trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
- "test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
-#line 33 "ftrace.h"
- trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
- unused = write(trace_marker_fd, ftrace_buf, trlen);
- }
+ ftrace_write("test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
+#line 28 "ftrace.h"
}
}
@@ -42,18 +34,10 @@ static inline void trace_test_blah(void *context, const char *filename)
static inline void trace_test_wibble(void *context, int value)
{
- {
- char ftrace_buf[MAX_TRACE_STRLEN];
- int unused __attribute__ ((unused));
- int trlen;
- if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
+ if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
#line 5 "trace-events"
- trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
- "test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
-#line 54 "ftrace.h"
- trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
- unused = write(trace_marker_fd, ftrace_buf, trlen);
- }
+ ftrace_write("test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
+#line 41 "ftrace.h"
}
}
#endif /* TRACE_TESTSUITE_GENERATED_TRACERS_H */
diff --git a/trace/ftrace.h b/trace/ftrace.h
index cb5e35d2171..16c122816d1 100644
--- a/trace/ftrace.h
+++ b/trace/ftrace.h
@@ -8,5 +8,6 @@
extern int trace_marker_fd;
bool ftrace_init(void);
+G_GNUC_PRINTF(1, 2) void ftrace_write(const char *fmt, ...);
#endif /* TRACE_FTRACE_H */
diff --git a/trace/ftrace.c b/trace/ftrace.c
index 9749543d9b2..6875faedb9c 100644
--- a/trace/ftrace.c
+++ b/trace/ftrace.c
@@ -38,6 +38,21 @@ static int find_mount(char *mount_point, const char *fstype)
return ret;
}
+void ftrace_write(const char *fmt, ...)
+{
+ char ftrace_buf[MAX_TRACE_STRLEN];
+ int unused __attribute__ ((unused));
+ int trlen;
+ va_list ap;
+
+ va_start(ap, fmt);
+ trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
+ va_end(ap);
+
+ trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
+ unused = write(trace_marker_fd, ftrace_buf, trlen);
+}
+
bool ftrace_init(void)
{
char mount_point[PATH_MAX];
diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
index 5fa30ccc08e..a07f8a9dfd8 100644
--- a/scripts/tracetool/backend/ftrace.py
+++ b/scripts/tracetool/backend/ftrace.py
@@ -28,18 +28,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(' if (trace_event_get_state(%(event_id)s)) {',
'#line %(event_lineno)d "%(event_filename)s"',
- ' trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
- ' "%(name)s " %(fmt)s "\\n" %(argnames)s);',
+ ' ftrace_write("%(name)s " %(fmt)s "\\n" %(argnames)s);',
'#line %(out_next_lineno)d "%(out_filename)s"',
- ' trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
- ' unused = write(trace_marker_fd, ftrace_buf, trlen);',
- ' }',
' }',
name=event.name,
args=event.args,
--
2.50.1On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote: > This simplifies the Python code and reduces the size of the tracepoints. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > tests/tracetool/ftrace.h | 28 ++++++---------------------- > trace/ftrace.h | 1 + > trace/ftrace.c | 15 +++++++++++++++ > scripts/tracetool/backend/ftrace.py | 12 ++---------- > 4 files changed, 24 insertions(+), 32 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote:
> This simplifies the Python code and reduces the size of the tracepoints.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> tests/tracetool/ftrace.h | 28 ++++++----------------------
> trace/ftrace.h | 1 +
> trace/ftrace.c | 15 +++++++++++++++
> scripts/tracetool/backend/ftrace.py | 12 ++----------
> 4 files changed, 24 insertions(+), 32 deletions(-)
>
> diff --git a/tests/tracetool/ftrace.h b/tests/tracetool/ftrace.h
> index fe22ea0f09f..1dfe4239413 100644
> --- a/tests/tracetool/ftrace.h
> +++ b/tests/tracetool/ftrace.h
> @@ -21,18 +21,10 @@ extern uint16_t _TRACE_TEST_WIBBLE_DSTATE;
>
> static inline void trace_test_blah(void *context, const char *filename)
> {
> - {
> - char ftrace_buf[MAX_TRACE_STRLEN];
> - int unused __attribute__ ((unused));
> - int trlen;
> - if (trace_event_get_state(TRACE_TEST_BLAH)) {
> + if (trace_event_get_state(TRACE_TEST_BLAH)) {
> #line 4 "trace-events"
> - trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
> - "test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
> -#line 33 "ftrace.h"
> - trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> - unused = write(trace_marker_fd, ftrace_buf, trlen);
> - }
> + ftrace_write("test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
> +#line 28 "ftrace.h"
> }
> }
>
> @@ -42,18 +34,10 @@ static inline void trace_test_blah(void *context, const char *filename)
>
> static inline void trace_test_wibble(void *context, int value)
> {
> - {
> - char ftrace_buf[MAX_TRACE_STRLEN];
> - int unused __attribute__ ((unused));
> - int trlen;
> - if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
> + if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
> #line 5 "trace-events"
> - trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
> - "test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
> -#line 54 "ftrace.h"
> - trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> - unused = write(trace_marker_fd, ftrace_buf, trlen);
> - }
> + ftrace_write("test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
> +#line 41 "ftrace.h"
> }
> }
> #endif /* TRACE_TESTSUITE_GENERATED_TRACERS_H */
snip
> diff --git a/trace/ftrace.c b/trace/ftrace.c
> index 9749543d9b2..6875faedb9c 100644
> --- a/trace/ftrace.c
> +++ b/trace/ftrace.c
> @@ -38,6 +38,21 @@ static int find_mount(char *mount_point, const char *fstype)
> return ret;
> }
>
> +void ftrace_write(const char *fmt, ...)
> +{
> + char ftrace_buf[MAX_TRACE_STRLEN];
> + int unused __attribute__ ((unused));
> + int trlen;
> + va_list ap;
> +
> + va_start(ap, fmt);
> + trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
> + va_end(ap);
> +
> + trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> + unused = write(trace_marker_fd, ftrace_buf, trlen);
You're just copying the existing code pattern which is fine for now so
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
More generally though, IMHO, QEMU would be better off bringing in
gnulib's 'ignore_value' macro, but simplified since we don't care
about ancient GCC
#define ignore_value(x) \
(__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))
so that we don't need to play games with extra variables. eg
ignore_value(write(trace_marker_fd, ftrace_buf, trlen));
With regards,
Daniel
[1] https://github.com/coreutils/gnulib/blob/master/lib/ignore-value.h#L38
--
|: 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 :|
On Tue, 26 Aug 2025 at 12:42, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote:
> > This simplifies the Python code and reduces the size of the tracepoints.
> > +void ftrace_write(const char *fmt, ...)
> > +{
> > + char ftrace_buf[MAX_TRACE_STRLEN];
> > + int unused __attribute__ ((unused));
> > + int trlen;
> > + va_list ap;
> > +
> > + va_start(ap, fmt);
> > + trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
> > + va_end(ap);
> > +
> > + trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> > + unused = write(trace_marker_fd, ftrace_buf, trlen);
>
> You're just copying the existing code pattern which is fine for now so
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> More generally though, IMHO, QEMU would be better off bringing in
> gnulib's 'ignore_value' macro, but simplified since we don't care
> about ancient GCC
>
> #define ignore_value(x) \
> (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))
>
> so that we don't need to play games with extra variables. eg
>
> ignore_value(write(trace_marker_fd, ftrace_buf, trlen));
Isn't there a way to write that that explicitly tells
the compiler "this is unused" (i.e. with the 'unused'
attribute) rather than relying on a particular construct
to not trigger a warning?
-- PMM
On Tue, Aug 26, 2025 at 12:45:42PM +0100, Peter Maydell wrote:
> On Tue, 26 Aug 2025 at 12:42, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote:
> > > This simplifies the Python code and reduces the size of the tracepoints.
>
> > > +void ftrace_write(const char *fmt, ...)
> > > +{
> > > + char ftrace_buf[MAX_TRACE_STRLEN];
> > > + int unused __attribute__ ((unused));
> > > + int trlen;
> > > + va_list ap;
> > > +
> > > + va_start(ap, fmt);
> > > + trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
> > > + va_end(ap);
> > > +
> > > + trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> > > + unused = write(trace_marker_fd, ftrace_buf, trlen);
> >
> > You're just copying the existing code pattern which is fine for now so
> >
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> >
> >
> > More generally though, IMHO, QEMU would be better off bringing in
> > gnulib's 'ignore_value' macro, but simplified since we don't care
> > about ancient GCC
> >
> > #define ignore_value(x) \
> > (__extension__ ({ __typeof__ (x) __x = (x); (void) __x; }))
> >
> > so that we don't need to play games with extra variables. eg
> >
> > ignore_value(write(trace_marker_fd, ftrace_buf, trlen));
>
> Isn't there a way to write that that explicitly tells
> the compiler "this is unused" (i.e. with the 'unused'
> attribute) rather than relying on a particular construct
> to not trigger a warning?
I think what the code is already doing is the closest to making it
explicit, but that needs the extra variable which is unpleasant.
It is annoyng that 'warn_unused_result' doesn't let you just cast
to (void) to discard a result without warnings :-(
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 :|
On Fri, Aug 22, 2025 at 02:26:44PM +0200, Paolo Bonzini wrote: > Date: Fri, 22 Aug 2025 14:26:44 +0200 > From: Paolo Bonzini <pbonzini@redhat.com> > Subject: [PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints > to ftrace.c > X-Mailer: git-send-email 2.50.1 > > This simplifies the Python code and reduces the size of the tracepoints. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > tests/tracetool/ftrace.h | 28 ++++++---------------------- > trace/ftrace.h | 1 + > trace/ftrace.c | 15 +++++++++++++++ > scripts/tracetool/backend/ftrace.py | 12 ++---------- > 4 files changed, 24 insertions(+), 32 deletions(-) Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
On Fri, Aug 22, 2025 at 3:28 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> This simplifies the Python code and reduces the size of the tracepoints.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
Nice.
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> tests/tracetool/ftrace.h | 28 ++++++----------------------
> trace/ftrace.h | 1 +
> trace/ftrace.c | 15 +++++++++++++++
> scripts/tracetool/backend/ftrace.py | 12 ++----------
> 4 files changed, 24 insertions(+), 32 deletions(-)
>
> diff --git a/tests/tracetool/ftrace.h b/tests/tracetool/ftrace.h
> index fe22ea0f09f..1dfe4239413 100644
> --- a/tests/tracetool/ftrace.h
> +++ b/tests/tracetool/ftrace.h
> @@ -21,18 +21,10 @@ extern uint16_t _TRACE_TEST_WIBBLE_DSTATE;
>
> static inline void trace_test_blah(void *context, const char *filename)
> {
> - {
> - char ftrace_buf[MAX_TRACE_STRLEN];
> - int unused __attribute__ ((unused));
> - int trlen;
> - if (trace_event_get_state(TRACE_TEST_BLAH)) {
> + if (trace_event_get_state(TRACE_TEST_BLAH)) {
> #line 4 "trace-events"
> - trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
> - "test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
> -#line 33 "ftrace.h"
> - trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> - unused = write(trace_marker_fd, ftrace_buf, trlen);
> - }
> + ftrace_write("test_blah " "Blah context=%p filename=%s" "\n" , context, filename);
> +#line 28 "ftrace.h"
> }
> }
>
> @@ -42,18 +34,10 @@ static inline void trace_test_blah(void *context, const char *filename)
>
> static inline void trace_test_wibble(void *context, int value)
> {
> - {
> - char ftrace_buf[MAX_TRACE_STRLEN];
> - int unused __attribute__ ((unused));
> - int trlen;
> - if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
> + if (trace_event_get_state(TRACE_TEST_WIBBLE)) {
> #line 5 "trace-events"
> - trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,
> - "test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
> -#line 54 "ftrace.h"
> - trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> - unused = write(trace_marker_fd, ftrace_buf, trlen);
> - }
> + ftrace_write("test_wibble " "Wibble context=%p value=%d" "\n" , context, value);
> +#line 41 "ftrace.h"
> }
> }
> #endif /* TRACE_TESTSUITE_GENERATED_TRACERS_H */
> diff --git a/trace/ftrace.h b/trace/ftrace.h
> index cb5e35d2171..16c122816d1 100644
> --- a/trace/ftrace.h
> +++ b/trace/ftrace.h
> @@ -8,5 +8,6 @@
> extern int trace_marker_fd;
>
> bool ftrace_init(void);
> +G_GNUC_PRINTF(1, 2) void ftrace_write(const char *fmt, ...);
>
> #endif /* TRACE_FTRACE_H */
> diff --git a/trace/ftrace.c b/trace/ftrace.c
> index 9749543d9b2..6875faedb9c 100644
> --- a/trace/ftrace.c
> +++ b/trace/ftrace.c
> @@ -38,6 +38,21 @@ static int find_mount(char *mount_point, const char *fstype)
> return ret;
> }
>
> +void ftrace_write(const char *fmt, ...)
> +{
> + char ftrace_buf[MAX_TRACE_STRLEN];
> + int unused __attribute__ ((unused));
> + int trlen;
> + va_list ap;
> +
> + va_start(ap, fmt);
> + trlen = vsnprintf(ftrace_buf, MAX_TRACE_STRLEN, fmt, ap);
> + va_end(ap);
> +
> + trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);
> + unused = write(trace_marker_fd, ftrace_buf, trlen);
> +}
> +
> bool ftrace_init(void)
> {
> char mount_point[PATH_MAX];
> diff --git a/scripts/tracetool/backend/ftrace.py b/scripts/tracetool/backend/ftrace.py
> index 5fa30ccc08e..a07f8a9dfd8 100644
> --- a/scripts/tracetool/backend/ftrace.py
> +++ b/scripts/tracetool/backend/ftrace.py
> @@ -28,18 +28,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(' if (trace_event_get_state(%(event_id)s)) {',
> '#line %(event_lineno)d "%(event_filename)s"',
> - ' trlen = snprintf(ftrace_buf, MAX_TRACE_STRLEN,',
> - ' "%(name)s " %(fmt)s "\\n" %(argnames)s);',
> + ' ftrace_write("%(name)s " %(fmt)s "\\n" %(argnames)s);',
> '#line %(out_next_lineno)d "%(out_filename)s"',
> - ' trlen = MIN(trlen, MAX_TRACE_STRLEN - 1);',
> - ' unused = write(trace_marker_fd, ftrace_buf, trlen);',
> - ' }',
> ' }',
> name=event.name,
> args=event.args,
> --
> 2.50.1
>
>
© 2016 - 2025 Red Hat, Inc.