On Fri, 26 Jan 2024 at 00:56, Joe Komlodi <koml...@google.com> wrote: > > The current logging doesn't tell us which specific smbus device is an > error state. > > Signed-off-by: Joe Komlodi <koml...@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