Hi Christophe,
Here's the new patch with the extra safety checks.
Cheers!Here's the new patch with the extra safety checks.
iordan
On Mon, Dec 9, 2013 at 11:56 AM, i iordanov <iiordanov@xxxxxxxxx> wrote:
Hi Christophe,On Mon, Dec 9, 2013 at 11:35 AM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
Ah cool, thanks. This mostly looks good, though I'd probably add a check
that the property has the right type using g_object_class_find_property.That's a good idea.
Hmm, I'm not sure about that one, what is the reasoning for it? I'd move
> I also switched out the two G_MAXUINT's in OvirtVmPool for G_MAXUINT32's.
this to a different commit.Fair point about the different commit. I was aligning them to your choice of making ovirt_utils_guint_from_string() return false if value64 exceeds G_MAXUINT32.
This is a quick change, so I'll get you a new patch shortly.
Thanks!iordan
--
The conscious mind has only one thread of execution.
From 04eadda83949733020a5cffc1dc26768063f31e9 Mon Sep 17 00:00:00 2001
From: Iordan Iordanov <iiordanov@xxxxxxxxx>
Date: Mon, 9 Dec 2013 12:23:43 -0500
Subject: [PATCH] Reduced code duplication in OvirtVmPool by adding a utility
function.
The functions to get size, prestarted_vms, and user_max_vms were all almost
the same code because each one was getting an integer from an XML node.
To deal with the code duplication, a utility function was added which
given a g_object and an XML node, sets a specified g_object property
from a specified XML sub-node. This patch removes the separate functions
and replaces them with calls to the new utility function, called
g_object_set_guint_property_from_xml().
---
govirt/ovirt-utils.c | 23 +++++++++++++++++++
govirt/ovirt-utils.h | 4 ++++
govirt/ovirt-vm-pool.c | 57 +++---------------------------------------------
3 files changed, 30 insertions(+), 54 deletions(-)
diff --git a/govirt/ovirt-utils.c b/govirt/ovirt-utils.c
index 3b9593a..dc4585e 100644
--- a/govirt/ovirt-utils.c
+++ b/govirt/ovirt-utils.c
@@ -212,3 +212,26 @@ G_GNUC_INTERNAL gboolean ovirt_utils_gerror_from_xml_fault(RestXmlNode *root, GE
return TRUE;
}
+
+
+G_GNUC_INTERNAL gboolean g_object_set_guint_property_from_xml(GObject *g_object,
+ RestXmlNode *node,
+ const gchar *node_name,
+ const gchar *prop_name)
+{
+ RestXmlNode *sub_node;
+ GParamSpec *spec;
+ sub_node = rest_xml_node_find(node, node_name);
+ if (sub_node != NULL && sub_node->content != NULL) {
+ guint value;
+ if (!ovirt_utils_guint_from_string(sub_node->content, &value)) {
+ return FALSE;
+ }
+ spec = g_object_class_find_property(G_OBJECT_GET_CLASS(g_object), prop_name);
+ if (spec != NULL && spec->value_type == G_TYPE_UINT) {
+ g_object_set(g_object, prop_name, value, NULL);
+ return TRUE;
+ }
+ }
+ return FALSE;
+}
diff --git a/govirt/ovirt-utils.h b/govirt/ovirt-utils.h
index f627c13..e92a2ec 100644
--- a/govirt/ovirt-utils.h
+++ b/govirt/ovirt-utils.h
@@ -32,6 +32,10 @@ G_BEGIN_DECLS
RestXmlNode *ovirt_rest_xml_node_from_call(RestProxyCall *call);
const char *ovirt_rest_xml_node_get_content(RestXmlNode *node, ...);
gboolean ovirt_utils_gerror_from_xml_fault(RestXmlNode *root, GError **error);
+gboolean g_object_set_guint_property_from_xml(GObject *g_object,
+ RestXmlNode *node,
+ const gchar *node_name,
+ const gchar *prop_name);
const char *ovirt_utils_genum_get_nick (GType enum_type, gint value);
int ovirt_utils_genum_get_value (GType enum_type, const char *nick,
diff --git a/govirt/ovirt-vm-pool.c b/govirt/ovirt-vm-pool.c
index ebbf3ae..61caafb 100644
--- a/govirt/ovirt-vm-pool.c
+++ b/govirt/ovirt-vm-pool.c
@@ -196,61 +196,10 @@ gboolean ovirt_vm_pool_allocate_vm_finish(OvirtVmPool *vm_pool,
}
-static gboolean vm_pool_set_size_from_xml(OvirtVmPool *vm_pool, RestXmlNode *node)
-{
- RestXmlNode *size_node;
- size_node = rest_xml_node_find(node, "size");
- if (size_node != NULL) {
- guint size;
- g_return_val_if_fail(size_node->content != NULL, FALSE);
- if (!ovirt_utils_guint_from_string(size_node->content, &size)) {
- return FALSE;
- }
- g_object_set(G_OBJECT(vm_pool), "size", size, NULL);
- return TRUE;
- }
- return FALSE;
-}
-
-
-static gboolean vm_pool_set_prestarted_vms_from_xml(OvirtVmPool *vm_pool, RestXmlNode *node)
-{
- RestXmlNode *prestarted_vms_node;
- prestarted_vms_node = rest_xml_node_find(node, "prestarted_vms");
- if (prestarted_vms_node != NULL) {
- guint prestarted_vms;
- g_return_val_if_fail(prestarted_vms_node->content != NULL, FALSE);
- if (!ovirt_utils_guint_from_string(prestarted_vms_node->content, &prestarted_vms)) {
- return FALSE;
- }
- g_object_set(G_OBJECT(vm_pool), "prestarted_vms", prestarted_vms, NULL);
- return TRUE;
- }
- return FALSE;
-}
-
-
-static gboolean vm_pool_set_max_user_vms_from_xml(OvirtVmPool *vm_pool, RestXmlNode *node)
-{
- RestXmlNode *max_user_vms_node;
- max_user_vms_node = rest_xml_node_find(node, "max_user_vms");
- if (max_user_vms_node != NULL) {
- guint max_user_vms;
- g_return_val_if_fail(max_user_vms_node->content != NULL, FALSE);
- if (!ovirt_utils_guint_from_string(max_user_vms_node->content, &max_user_vms)) {
- return FALSE;
- }
- g_object_set(G_OBJECT(vm_pool), "max_user_vms", max_user_vms, NULL);
- return TRUE;
- }
- return FALSE;
-}
-
-
static gboolean ovirt_vm_pool_refresh_from_xml(OvirtVmPool *vm_pool, RestXmlNode *node)
{
- vm_pool_set_size_from_xml(vm_pool, node);
- vm_pool_set_prestarted_vms_from_xml(vm_pool, node);
- vm_pool_set_max_user_vms_from_xml(vm_pool, node);
+ g_object_set_guint_property_from_xml(G_OBJECT(vm_pool), node, "size", "size");
+ g_object_set_guint_property_from_xml(G_OBJECT(vm_pool), node, "prestarted_vms", "prestarted_vms");
+ g_object_set_guint_property_from_xml(G_OBJECT(vm_pool), node, "max_user_vms", "max_user_vms");
return TRUE;
}
--
1.7.9.5
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel