[PATCH] hw/xen: Fix trace_xs_node_read() params

[PATCH] hw/xen: Fix trace_xs_node_read() params

[Liam Merwick]

When the '--enable-trace-backends=syslog' build option is configured,
the following compilation error is encountered.

In file included from /usr/include/sys/syslog.h:207,
                 from /usr/include/syslog.h:1,
                 from ./trace/trace-hw_xen.h:224,
                 from ../hw/xen/trace.h:1,
                 from ../hw/xen/xen-bus-helper.c:13:
In function ‘syslog’,
    inlined from ‘_nocheck__trace_xs_node_read’ at ../hw/xen/trace-events:41:9,
    inlined from ‘trace_xs_node_read’ at trace/trace-hw_xen.h:903:9,
    inlined from ‘xs_node_read’ at ../hw/xen/xen-bus-helper.c:154:5:
/usr/include/bits/syslog.h:45:3: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
   45 |   __syslog_chk (__pri, __USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Add a check that 'value' is not null before passing it to trace_xs_node_read().

Fixes: e6cdeee95990 ("hw/xen: Add xs_node_read() helper function")
Signed-off-by: Liam Merwick <liam.merwick@oracle.com>
---
 hw/xen/xen-bus-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/xen/xen-bus-helper.c b/hw/xen/xen-bus-helper.c
index 288fad422be3..1087a585cc71 100644
--- a/hw/xen/xen-bus-helper.c
+++ b/hw/xen/xen-bus-helper.c
@@ -151,7 +151,7 @@ char *xs_node_read(struct qemu_xs_handle *h, xs_transaction_t tid,
     va_end(ap);
 
     value = qemu_xen_xs_read(h, tid, path, len);
-    trace_xs_node_read(path, value);
+    trace_xs_node_read(path, value ? value : "<null>");
     if (!value) {
         error_setg_errno(errp, errno, "failed to read from '%s'", path);
     }
-- 
2.47.1

Re: [EXTERNAL] [PATCH] hw/xen: Fix trace_xs_node_read() params

[David Woodhouse]

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

On Fri, 2025-05-23 at 16:01 +0000, Liam Merwick wrote:
> When the '--enable-trace-backends=syslog' build option is configured,
> the following compilation error is encountered.
> 
> In file included from /usr/include/sys/syslog.h:207,
>                  from /usr/include/syslog.h:1,
>                  from ./trace/trace-hw_xen.h:224,
>                  from ../hw/xen/trace.h:1,
>                  from ../hw/xen/xen-bus-helper.c:13:
> In function ‘syslog’,
>     inlined from ‘_nocheck__trace_xs_node_read’ at ../hw/xen/trace-events:41:9,
>     inlined from ‘trace_xs_node_read’ at trace/trace-hw_xen.h:903:9,
>     inlined from ‘xs_node_read’ at ../hw/xen/xen-bus-helper.c:154:5:
> /usr/include/bits/syslog.h:45:3: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
>    45 |   __syslog_chk (__pri, __USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
>       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Add a check that 'value' is not null before passing it to trace_xs_node_read().
> 
> Fixes: e6cdeee95990 ("hw/xen: Add xs_node_read() helper function")
> Signed-off-by: Liam Merwick <liam.merwick@oracle.com>

Acked-by: David Woodhouse <dwmw@amazon.co.uk>

Thanks.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH] hw/xen: Fix trace_xs_node_read() params

[Anthony PERARD]

On Fri, May 23, 2025 at 04:01:34PM +0000, Liam Merwick wrote:
> When the '--enable-trace-backends=syslog' build option is configured,
> the following compilation error is encountered.
> 
> In file included from /usr/include/sys/syslog.h:207,
>                  from /usr/include/syslog.h:1,
>                  from ./trace/trace-hw_xen.h:224,
>                  from ../hw/xen/trace.h:1,
>                  from ../hw/xen/xen-bus-helper.c:13:
> In function ‘syslog’,
>     inlined from ‘_nocheck__trace_xs_node_read’ at ../hw/xen/trace-events:41:9,
>     inlined from ‘trace_xs_node_read’ at trace/trace-hw_xen.h:903:9,
>     inlined from ‘xs_node_read’ at ../hw/xen/xen-bus-helper.c:154:5:
> /usr/include/bits/syslog.h:45:3: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
>    45 |   __syslog_chk (__pri, __USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
>       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Add a check that 'value' is not null before passing it to trace_xs_node_read().
> 
> Fixes: e6cdeee95990 ("hw/xen: Add xs_node_read() helper function")
> Signed-off-by: Liam Merwick <liam.merwick@oracle.com>

Acked-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD

Re: [PATCH] hw/xen: Fix trace_xs_node_read() params

[Philippe Mathieu-Daudé]

On 23/5/25 18:01, Liam Merwick wrote:
> When the '--enable-trace-backends=syslog' build option is configured,
> the following compilation error is encountered.
> 
> In file included from /usr/include/sys/syslog.h:207,
>                   from /usr/include/syslog.h:1,
>                   from ./trace/trace-hw_xen.h:224,
>                   from ../hw/xen/trace.h:1,
>                   from ../hw/xen/xen-bus-helper.c:13:
> In function ‘syslog’,
>      inlined from ‘_nocheck__trace_xs_node_read’ at ../hw/xen/trace-events:41:9,
>      inlined from ‘trace_xs_node_read’ at trace/trace-hw_xen.h:903:9,
>      inlined from ‘xs_node_read’ at ../hw/xen/xen-bus-helper.c:154:5:
> /usr/include/bits/syslog.h:45:3: error: ‘%s’ directive argument is null [-Werror=format-overflow=]
>     45 |   __syslog_chk (__pri, __USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
>        |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Add a check that 'value' is not null before passing it to trace_xs_node_read().
> 
> Fixes: e6cdeee95990 ("hw/xen: Add xs_node_read() helper function")
> Signed-off-by: Liam Merwick <liam.merwick@oracle.com>
> ---
>   hw/xen/xen-bus-helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/xen/xen-bus-helper.c b/hw/xen/xen-bus-helper.c
> index 288fad422be3..1087a585cc71 100644
> --- a/hw/xen/xen-bus-helper.c
> +++ b/hw/xen/xen-bus-helper.c
> @@ -151,7 +151,7 @@ char *xs_node_read(struct qemu_xs_handle *h, xs_transaction_t tid,
>       va_end(ap);
>   
>       value = qemu_xen_xs_read(h, tid, path, len);
> -    trace_xs_node_read(path, value);
> +    trace_xs_node_read(path, value ? value : "<null>");
>       if (!value) {
>           error_setg_errno(errp, errno, "failed to read from '%s'", path);
>       }

Alternatively, since this is an error path:

-- >8 --
diff --git a/hw/xen/xen-bus-helper.c b/hw/xen/xen-bus-helper.c
index 288fad422be..1e49e60e147 100644
--- a/hw/xen/xen-bus-helper.c
+++ b/hw/xen/xen-bus-helper.c
@@ -143,7 +143,8 @@ char *xs_node_read(struct qemu_xs_handle *h, 
xs_transaction_t tid,
                     unsigned int *len, Error **errp,
                     const char *path_fmt, ...)
  {
-    char *path, *value;
+    g_autofree char *value;
+    char *path;
      va_list ap;

      va_start(ap, path_fmt);
@@ -151,12 +152,11 @@ char *xs_node_read(struct qemu_xs_handle *h, 
xs_transaction_t tid,
      va_end(ap);

      value = qemu_xen_xs_read(h, tid, path, len);
-    trace_xs_node_read(path, value);
      if (!value) {
          error_setg_errno(errp, errno, "failed to read from '%s'", path);
+        return NULL;
      }
-
-    g_free(path);
+    trace_xs_node_read(path, value);

      return value;
  }
---

But your patch isn't wrong, so:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] hw/xen: Fix trace_xs_node_read() params

[Richard Henderson]

On 5/26/25 19:10, Philippe Mathieu-Daudé wrote:
> Alternatively, since this is an error path:
> 
> -- >8 --
> diff --git a/hw/xen/xen-bus-helper.c b/hw/xen/xen-bus-helper.c
> index 288fad422be..1e49e60e147 100644
> --- a/hw/xen/xen-bus-helper.c
> +++ b/hw/xen/xen-bus-helper.c
> @@ -143,7 +143,8 @@ char *xs_node_read(struct qemu_xs_handle *h, xs_transaction_t tid,
>                      unsigned int *len, Error **errp,
>                      const char *path_fmt, ...)
>   {
> -    char *path, *value;
> +    g_autofree char *value;
> +    char *path;
>       va_list ap;
> 
>       va_start(ap, path_fmt);
> @@ -151,12 +152,11 @@ char *xs_node_read(struct qemu_xs_handle *h, xs_transaction_t tid,
>       va_end(ap);
> 
>       value = qemu_xen_xs_read(h, tid, path, len);
> -    trace_xs_node_read(path, value);
>       if (!value) {
>           error_setg_errno(errp, errno, "failed to read from '%s'", path);
> +        return NULL;
>       }

Auto-free variables must be initialized.


r~