1. Patchew
  2. Libvirt
  3. View series

[PATCH] src: Fix return types of .stateInitialize callbacks

Michal Privoznik posted 1 patch 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/3fd9747bb42963c692be2c6d999b22b189999fa2.1716372495.git.mprivozn@redhat.com
src/bhyve/bhyve_driver.c                |  2 +-
src/ch/ch_driver.c                      | 11 ++++++-----
src/interface/interface_backend_netcf.c |  2 +-
src/interface/interface_backend_udev.c  |  2 +-
src/libxl/libxl_driver.c                |  2 +-
src/lxc/lxc_driver.c                    | 11 ++++++-----
src/network/bridge_driver.c             |  2 +-
src/node_device/node_device_udev.c      |  2 +-
src/nwfilter/nwfilter_driver.c          |  2 +-
src/qemu/qemu_driver.c                  |  2 +-
src/remote/remote_driver.c              |  2 +-
src/secret/secret_driver.c              |  2 +-
src/storage/storage_driver.c            |  2 +-
src/vz/vz_driver.c                      |  2 +-
14 files changed, 24 insertions(+), 22 deletions(-)
[PATCH] src: Fix return types of .stateInitialize callbacks
Posted by Michal Privoznik 6 months ago 
The virStateDriver struct has .stateInitialize callback which is
declared to return virDrvStateInitResult enum. But some drivers
return a plain int in their implementation which is UB.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/bhyve/bhyve_driver.c                |  2 +-
 src/ch/ch_driver.c                      | 11 ++++++-----
 src/interface/interface_backend_netcf.c |  2 +-
 src/interface/interface_backend_udev.c  |  2 +-
 src/libxl/libxl_driver.c                |  2 +-
 src/lxc/lxc_driver.c                    | 11 ++++++-----
 src/network/bridge_driver.c             |  2 +-
 src/node_device/node_device_udev.c      |  2 +-
 src/nwfilter/nwfilter_driver.c          |  2 +-
 src/qemu/qemu_driver.c                  |  2 +-
 src/remote/remote_driver.c              |  2 +-
 src/secret/secret_driver.c              |  2 +-
 src/storage/storage_driver.c            |  2 +-
 src/vz/vz_driver.c                      |  2 +-
 14 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 4203b13f94..2bd1e4c387 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1176,7 +1176,7 @@ bhyveStateCleanup(void)
     return 0;
 }
 
-static int
+static virDrvStateInitResult
 bhyveStateInitialize(bool privileged,
                      const char *root,
                      bool monolithic G_GNUC_UNUSED,
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 7308f40249..fbeac60825 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -1365,11 +1365,12 @@ static int chStateCleanup(void)
     return 0;
 }
 
-static int chStateInitialize(bool privileged,
-                             const char *root,
-                             bool monolithic G_GNUC_UNUSED,
-                             virStateInhibitCallback callback G_GNUC_UNUSED,
-                             void *opaque G_GNUC_UNUSED)
+static virDrvStateInitResult
+chStateInitialize(bool privileged,
+                  const char *root,
+                  bool monolithic G_GNUC_UNUSED,
+                  virStateInhibitCallback callback G_GNUC_UNUSED,
+                  void *opaque G_GNUC_UNUSED)
 {
     int ret = VIR_DRV_STATE_INIT_ERROR;
     int rv;
diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c
index d4a11157cc..16e1215663 100644
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -86,7 +86,7 @@ virNetcfDriverStateDispose(void *obj)
 }
 
 
-static int
+static virDrvStateInitResult
 netcfStateInitialize(bool privileged,
                      const char *root,
                      bool monolithic G_GNUC_UNUSED,
diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
index 8bb19d7764..fdf11a8318 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -1091,7 +1091,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo)
 static int
 udevStateCleanup(void);
 
-static int
+static virDrvStateInitResult
 udevStateInitialize(bool privileged,
                     const char *root,
                     bool monolithic G_GNUC_UNUSED,
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 4d5eb920bf..7dcae58413 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -645,7 +645,7 @@ libxlAddDom0(libxlDriverPrivate *driver)
     return ret;
 }
 
-static int
+static virDrvStateInitResult
 libxlStateInitialize(bool privileged,
                      const char *root,
                      bool monolithic G_GNUC_UNUSED,
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 1842ae8f0e..f76d09e8a9 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1429,11 +1429,12 @@ lxcSecurityInit(virLXCDriverConfig *cfg)
 }
 
 
-static int lxcStateInitialize(bool privileged,
-                              const char *root,
-                              bool monolithic G_GNUC_UNUSED,
-                              virStateInhibitCallback callback G_GNUC_UNUSED,
-                              void *opaque G_GNUC_UNUSED)
+static virDrvStateInitResult
+lxcStateInitialize(bool privileged,
+                   const char *root,
+                   bool monolithic G_GNUC_UNUSED,
+                   virStateInhibitCallback callback G_GNUC_UNUSED,
+                   void *opaque G_GNUC_UNUSED)
 {
     virLXCDriverConfig *cfg = NULL;
     bool autostart = true;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e5f9ecf9e8..d7c1ef172d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -576,7 +576,7 @@ firewalld_dbus_signal_callback(GDBusConnection *connection G_GNUC_UNUSED,
  *
  * Initialization function for the QEMU daemon
  */
-static int
+static virDrvStateInitResult
 networkStateInitialize(bool privileged,
                        const char *root,
                        bool monolithic G_GNUC_UNUSED,
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index f1e402f8f7..237cd7f645 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -2229,7 +2229,7 @@ mdevctlEventHandleCallback(GFileMonitor *monitor G_GNUC_UNUSED,
 }
 
 
-static int
+static virDrvStateInitResult
 nodeStateInitialize(bool privileged,
                     const char *root,
                     bool monolithic G_GNUC_UNUSED,
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 09719edd75..8ece91bf7c 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -208,7 +208,7 @@ nwfilterStateCleanup(void)
  *
  * Initialization function for the QEMU daemon
  */
-static int
+static virDrvStateInitResult
 nwfilterStateInitialize(bool privileged,
                         const char *root,
                         bool monolithic G_GNUC_UNUSED,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d01f788aea..e2698c7924 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -546,7 +546,7 @@ qemuDomainFindMaxID(virDomainObj *vm,
  *
  * Initialization function for the QEMU daemon
  */
-static int
+static virDrvStateInitResult
 qemuStateInitialize(bool privileged,
                     const char *root,
                     bool monolithic G_GNUC_UNUSED,
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 7b73d97b7a..e76d9e9ba4 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -174,7 +174,7 @@ static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapsho
 /* Helper functions for remoteOpen. */
 
 
-static int
+static virDrvStateInitResult
 remoteStateInitialize(bool privileged G_GNUC_UNUSED,
                       const char *root G_GNUC_UNUSED,
                       bool monolithic,
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index c7bd65b4e9..a2d6b879d0 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -488,7 +488,7 @@ secretStateCleanup(void)
 }
 
 
-static int
+static virDrvStateInitResult
 secretStateInitialize(bool privileged,
                       const char *root,
                       bool monolithic G_GNUC_UNUSED,
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 314fe930e0..86c03762d2 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -239,7 +239,7 @@ storageDriverAutostart(void)
  *
  * Initialization function for the Storage Driver
  */
-static int
+static virDrvStateInitResult
 storageStateInitialize(bool privileged,
                        const char *root,
                        bool monolithic G_GNUC_UNUSED,
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 380fdcb57e..4edea4bf18 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -4068,7 +4068,7 @@ vzStateCleanup(void)
     return 0;
 }
 
-static int
+static virDrvStateInitResult
 vzStateInitialize(bool privileged,
                   const char *root,
                   bool monolithic G_GNUC_UNUSED,
-- 
2.44.1
Re: [PATCH] src: Fix return types of .stateInitialize callbacks
Posted by Daniel P. Berrangé 6 months ago 
On Wed, May 22, 2024 at 12:08:15PM +0200, Michal Privoznik wrote:
> The virStateDriver struct has .stateInitialize callback which is
> declared to return virDrvStateInitResult enum. But some drivers
> return a plain int in their implementation which is UB.

Any idea if there's a warning flag we could potentially turn on that
would diagnose such a mis-match ?


Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: 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 :|
Re: [PATCH] src: Fix return types of .stateInitialize callbacks
Posted by Michal Prívozník 6 months ago 
On 5/22/24 12:22, Daniel P. Berrangé wrote:
> On Wed, May 22, 2024 at 12:08:15PM +0200, Michal Privoznik wrote:
>> The virStateDriver struct has .stateInitialize callback which is
>> declared to return virDrvStateInitResult enum. But some drivers
>> return a plain int in their implementation which is UB.
> 
> Any idea if there's a warning flag we could potentially turn on that
> would diagnose such a mis-match ?
> 

That's a good question. Similarly, I'm surprised neither gcc nor clang
complains when a function is declared to return an enum but there's
"return -20" somewhere (and -20 is NOT in the enum definition). I
haven't found any such warning. These were spotted when trying to start
libvirtd built with UBSAN.

> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Merged, thanks.

Michal

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.