Re: [PATCH 03/14] trace/ftrace: move snprintf+write from tracepoints to ftrace.c

["Daniel P. Berrangé" <berrange@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 :|