On Fri, 26 Jan 2024 at 00:56, Joe Komlodi <komlodi@google.com> wrote:
>
> The current logging doesn't tell us which specific smbus device is an
> error state.
>
> Signed-off-by: Joe Komlodi <komlodi@google.com>
> ---
> hw/i2c/smbus_slave.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c
> index 1300c9ec72..e24a1ef472 100644
> --- a/hw/i2c/smbus_slave.c
> +++ b/hw/i2c/smbus_slave.c
> @@ -25,11 +25,15 @@
> #define DPRINTF(fmt, ...) \
> do { printf("smbus(%02x): " fmt , dev->i2c.address, ## __VA_ARGS__); } while (0)
> #define BADF(fmt, ...) \
> -do { fprintf(stderr, "smbus: error: " fmt , ## __VA_ARGS__); exit(1);} while (0)
> +do { fprintf(stderr, "%s: smbus: error: " fmt , \
> + object_get_canonical_path(OBJECT(dev)), ## __VA_ARGS__); \
> + exit(1); } while (0)
> #else
> #define DPRINTF(fmt, ...) do {} while(0)
> #define BADF(fmt, ...) \
> -do { fprintf(stderr, "smbus: error: " fmt , ## __VA_ARGS__);} while (0)
> +do { fprintf(stderr, "%s: smbus: error: " fmt , \
> + object_get_canonical_path(OBJECT(dev)), ## __VA_ARGS__); \
> + } while (0)
> #endif
Ideally the uses of these macros should all be
tracepoints or uses of qemu_log_mask(LOG_GUEST_ERROR, ...),
but I'm OK with just making a minor improvement to the
existing macros.
However, object_get_canonical_path() returns a pointer
to allocated memory which the caller needs to free, so
this is leaking memory. You can fix that by making the
macros something like
#define BADF(fmt, ...) \
do { \
g_autofree char *qom_path = object_get_canonical_path(OBJECT(dev)); \
fprintf(stderr, etc); \
} while (0)
The g_autofree will arrange for the memory to be freed when
execution leaves the {} block.
thanks
-- PMM