Re: [PATCH] domain_conf: make virDomainDiskSetSource() void

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/5/20 11:26 AM, Michal Prívozník wrote:
On 11/5/20 7:23 AM, Matt Coleman wrote:
The function only returns zero or aborts, so it might as well be void.
This has the added benefit of simplifying the code that calls it.

Signed-off-by: Matt Coleman <matt@xxxxxxxxx>
---
  src/conf/domain_conf.c   |  3 +--
  src/conf/domain_conf.h   |  3 +--
  src/libxl/libxl_driver.c |  6 ++----
  src/libxl/xen_xl.c       |  3 +--
  src/libxl/xen_xm.c       | 13 ++++---------
  src/lxc/lxc_controller.c | 13 ++-----------
  src/vbox/vbox_common.c   |  6 +-----
  src/vmx/vmx.c            | 36 ++++++++++++------------------------
  src/vz/vz_sdk.c          |  4 ++--
  9 files changed, 26 insertions(+), 61 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7696b12ef9..5c8ec19da8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2241,13 +2241,12 @@ virDomainDiskGetSource(virDomainDiskDef const *def)
  }
    -int
+void
  virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src)
  {
      char *tmp = g_strdup(src);
      g_free(def->src->path);
      def->src->path = tmp;
-    return 0;
  }
    diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8a487e9de3..c164b28ea1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3040,8 +3040,7 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def);
  int virDomainDiskGetType(virDomainDiskDefPtr def);
  void virDomainDiskSetType(virDomainDiskDefPtr def, int type);
  const char *virDomainDiskGetSource(virDomainDiskDef const *def);
-int virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src)
-    G_GNUC_WARN_UNUSED_RESULT;
+void virDomainDiskSetSource(virDomainDiskDefPtr def, const char *src);

With us adopting glib more and more cleanups like these will be desirable more. Any plans on continuing?


I've done it as a part of other simplifications when possible, but there have been a few cases where a function is defined differently for different platforms, and on some platforms it just always returns an error - if we changed one of those functions to return void and there was ever a case where it ended up getting called on a platform where it should be returning an error, then the user would'nt get the notification that [whatever operation] depended on [whatever capability] that isn't supported on their platform.


I think it might be better to have those functions just *not defined at all* on platforms where they're not supported, but that would mean cleaning up the code at the next higher level, possibly polluting it with #ifdefs in the middle of functions.


Anyway, I'm all for doing this, just wanted to warn about these special cases.





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux