Re: [libvirt] [PATCH] udev: fix crash in libudev logging

----- Original Message ----- From: "Ján Tomko" <jtomko(a)redhat.com&gt; To: libvir-list(a)redhat.com Sent: Friday, June 7, 2013 7:20:10 AM Subject: [libvirt] [PATCH] udev: fix crash in libudev logging VIR_ERROR_INT calls virLogMessage(..., const char *fmt, ...). Call virLogVMessage(..., const char *fmt, va_list list) instead, since libudev called us with a va_list object, not a list of arguments. https://bugzilla.redhat.com/show_bug.cgi?id=969152 --- Without the cast, I was getting: node_device/node_device_udev.c: In function 'nodeStateInitialize': node_device/node_device_udev.c:1684:5: error: argument 2 of 'udev_set_log_fn' might be a candidate for a format attribute [-Werror=missing-format-attribute] node_device/node_device_udev.c: At top level: cc1: error: unrecognized command line option "-Wno-unused-command-line-argument" [-Werror] Is there a nicer way to get rid of it? src/libvirt_private.syms | 1 + src/node_device/node_device_udev.c | 30 +++++++++++++++++++++--------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b93629f..d73a4d0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1486,6 +1486,7 @@ virLogSetBufferSize; virLogSetDefaultPriority; virLogSetFromEnv; virLogUnlock; +virLogVMessage; # util/virmacaddr.h diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 620cd58..ffcae16 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -348,15 +348,26 @@ static int udevGenerateDeviceName(struct udev_device *device, } -static void udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED, - int priority ATTRIBUTE_UNUSED, - const char *file, - int line, - const char *fn, - const char *fmt, - va_list args) +typedef void (*udevLogFunctionPtr) (struct udev *udev, + int priority, + const char *file, + int line, + const char *fn, + const char *format, + va_list args); + +static void +ATTRIBUTE_FMT_PRINTF(6,0) +udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED, + int priority ATTRIBUTE_UNUSED, + const char *file, + int line, + const char *fn, + const char *fmt, + va_list args) { - VIR_ERROR_INT(VIR_LOG_FROM_LIBRARY, file, line, fn, fmt, args); + virLogVMessage(VIR_LOG_FROM_LIBRARY, VIR_LOG_ERROR, + file, line, fn, NULL, fmt, args); } @@ -1672,7 +1683,8 @@ static int nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED, * its return value. */ udev = udev_new(); - udev_set_log_fn(udev, udevLogFunction); + /* cast to get rid of missing-format-attribute warning */ + udev_set_log_fn(udev, (udevLogFunctionPtr) udevLogFunction); priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev"); if (priv->udev_monitor == NULL) { -- I have been trying to fix this bug these days, but I failed to find a good way to bypass the warning you mentioned above caused by a gcc bug: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28492 I can not find a better way than the casting in your code to compress the warning. other than this, there are some points needed to pay attention. 1, the log string output from udev ends with newline character which we could remove. 2, we can use debug mode for these logs. The following is the code with my code squashed in your casting code to fix this bug diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 620cd58..dc989e9 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -348,15 +348,32 @@ static int udevGenerateDeviceName(struct udev_device *device, } -static void udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED, - int priority ATTRIBUTE_UNUSED, - const char *file, - int line, - const char *fn, - const char *fmt, - va_list args) +typedef void (*udevLogFunctionPtr)(struct udev *udev, + int priority, + const char *file, + int line, + const char *fn, + const char *format, + va_list args); + +static void +ATTRIBUTE_FMT_PRINTF(6,0) +udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED, + int priority ATTRIBUTE_UNUSED, + const char *file ATTRIBUTE_UNUSED, + int line ATTRIBUTE_UNUSED, + const char *fn ATTRIBUTE_UNUSED, + const char *fmt, + va_list args) { - VIR_ERROR_INT(VIR_LOG_FROM_LIBRARY, file, line, fn, fmt, args); + char *eol = NULL; + char buffer[LINE_MAX]; + vsnprintf(buffer, sizeof(buffer), fmt, args); + + if ((eol = strpbrk(buffer, "\n\r"))) + *(eol++) = 0; + + VIR_DEBUG("%s", buffer); } @@ -1672,7 +1689,8 @@ static int nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED, * its return value. */ udev = udev_new(); - udev_set_log_fn(udev, udevLogFunction); + /* cast to get rid of missing-format-attribute warning */ + udev_set_log_fn(udev, (udevLogFunctionPtr) udevLogFunction); priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev"); if (priv->udev_monitor == NULL) {