[libvirt] [libvirt-glib 0/4] gconfig: Leak fixes and small cleanup

Hey, This patch series makes tests/test-gconfig valgrind-clean, and refactors two setters in GVirConfigDomainVideo to make them use the helpers provided by GVirConfigObject. Christophe

The object returned by gvir_config_object_replace_child() must be unref'ed when no longer needed. --- libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 860480c..97b7bd6 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c @@ -201,7 +201,7 @@ void gvir_config_domain_filesys_set_ram_usage(GVirConfigDomainFilesys *filesys, "usage", G_TYPE_UINT64, bytes, "units", G_TYPE_STRING, "bytes", NULL); - + g_object_unref(G_OBJECT(src)); } void gvir_config_domain_filesys_set_target(GVirConfigDomainFilesys *filesys, -- 2.4.3

Running test-gconfig under valgrind reports a few leaks that this commit fixes. --- tests/test-gconfig.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c index 0eec53e..de09c24 100644 --- a/tests/test-gconfig.c +++ b/tests/test-gconfig.c @@ -48,6 +48,7 @@ static void check_xml(GVirConfigDomain *domain, const char *reference_file) xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(domain)); g_assert_cmpstr(xml, ==, reference_xml); g_free(xml); + g_free(reference_xml); } @@ -224,6 +225,8 @@ static void test_domain_os(void) gvir_config_domain_set_os(domain, NULL); os = gvir_config_domain_get_os(domain); g_assert(os == NULL); + + g_object_unref(G_OBJECT(domain)); } @@ -294,9 +297,13 @@ static void test_domain_cpu(void) NULL); topology = gvir_config_capabilities_cpu_get_topology(GVIR_CONFIG_CAPABILITIES_CPU(cpu)); g_assert(topology == NULL); + g_object_unref(G_OBJECT(cpu)); + gvir_config_domain_set_cpu(domain, NULL); cpu = gvir_config_domain_get_cpu(domain); g_assert(cpu == NULL); + + g_object_unref(G_OBJECT(domain)); } @@ -347,6 +354,7 @@ static void test_domain_device_disk(void) g_assert(gvir_config_domain_disk_driver_get_copy_on_read(driver)); g_assert_cmpint(gvir_config_domain_disk_get_target_bus(disk), ==, GVIR_CONFIG_DOMAIN_DISK_BUS_IDE); g_assert_cmpstr(gvir_config_domain_disk_get_target_dev(disk), ==, "hda"); + g_object_unref(G_OBJECT(driver)); gvir_config_domain_disk_set_driver(disk, NULL); driver = gvir_config_domain_disk_get_driver(disk); @@ -433,6 +441,7 @@ static void test_domain_device_input(void) GVIR_CONFIG_DOMAIN_INPUT_DEVICE_TABLET); gvir_config_domain_input_set_bus(input, GVIR_CONFIG_DOMAIN_INPUT_BUS_USB); gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE(input)); + g_object_unref(G_OBJECT(input)); check_xml(domain, "gconfig-domain-device-input.xml"); -- 2.4.3

--- tests/test-gconfig.c | 2 ++ tests/xml/gconfig-domain-device-video.xml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c index de09c24..aca8f02 100644 --- a/tests/test-gconfig.c +++ b/tests/test-gconfig.c @@ -494,6 +494,8 @@ static void test_domain_device_video(void) video = gvir_config_domain_video_new(); gvir_config_domain_video_set_model(video, GVIR_CONFIG_DOMAIN_VIDEO_MODEL_QXL); + gvir_config_domain_video_set_heads(video, 4); + gvir_config_domain_video_set_vram(video, 256*1024); gvir_config_domain_add_device(domain, GVIR_CONFIG_DOMAIN_DEVICE(video)); g_object_unref(G_OBJECT(video)); diff --git a/tests/xml/gconfig-domain-device-video.xml b/tests/xml/gconfig-domain-device-video.xml index a6155e2..7af6256 100644 --- a/tests/xml/gconfig-domain-device-video.xml +++ b/tests/xml/gconfig-domain-device-video.xml @@ -1,7 +1,7 @@ <domain> <devices> <video> - <model type="qxl"/> + <model type="qxl" heads="4" vram="262144"/> </video> </devices> </domain> -- 2.4.3

GVirConfigDomainVideo is using raw libxml calls to set the 'heads' and 'vram' XML attributes rather than the helpers provided by GVirConfigObject. This commit changes that, making the code a bit simpler. --- libvirt-gconfig/libvirt-gconfig-domain-video.c | 38 +++++++++++--------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-video.c b/libvirt-gconfig/libvirt-gconfig-domain-video.c index 947d066..78ac54f 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-video.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-video.c @@ -88,33 +88,27 @@ void gvir_config_domain_video_set_model(GVirConfigDomainVideo *video, void gvir_config_domain_video_set_vram(GVirConfigDomainVideo *video, guint kbytes) { - xmlNodePtr node; - char *vram_str; + GVirConfigObject *node; - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(video)); - if (node == NULL) - return; - node = gvir_config_xml_get_element(node, "model", NULL); - if (node == NULL) - return; - vram_str = g_strdup_printf("%u", kbytes); - xmlNewProp(node, (xmlChar*)"vram", (xmlChar*)vram_str); - g_free(vram_str); + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_VIDEO(video)); + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(video), "model"); + g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); + gvir_config_object_set_attribute_with_type(node, "vram", + G_TYPE_UINT, kbytes, + NULL); + g_object_unref(G_OBJECT(node)); } void gvir_config_domain_video_set_heads(GVirConfigDomainVideo *video, guint head_count) { - xmlNodePtr node; - char *heads_str; + GVirConfigObject *node; - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(video)); - if (node == NULL) - return; - node = gvir_config_xml_get_element(node, "model", NULL); - if (node == NULL) - return; - heads_str = g_strdup_printf("%u", head_count); - xmlNewProp(node, (xmlChar*)"heads", (xmlChar*)heads_str); - g_free(heads_str); + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_VIDEO(video)); + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(video), "model"); + g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); + gvir_config_object_set_attribute_with_type(node, "heads", + G_TYPE_UINT, head_count, + NULL); + g_object_unref(G_OBJECT(node)); } -- 2.4.3

Hey, On Fri, Jul 17, 2015 at 09:01:46AM +0200, Christophe Fergeau wrote:
On Thu, Jul 09, 2015 at 10:13:16AM +0200, Christophe Fergeau wrote:
Hey,
This patch series makes tests/test-gconfig valgrind-clean, and refactors two setters in GVirConfigDomainVideo to make them use the helpers provided by GVirConfigObject.
Ping?
These missed the libvirt-glib release, it would be nice to get them in... :) Christophe

On Wed, Jul 22, 2015 at 11:41 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Hey,
On Fri, Jul 17, 2015 at 09:01:46AM +0200, Christophe Fergeau wrote:
On Thu, Jul 09, 2015 at 10:13:16AM +0200, Christophe Fergeau wrote:
Hey,
This patch series makes tests/test-gconfig valgrind-clean, and refactors two setters in GVirConfigDomainVideo to make them use the helpers provided by GVirConfigObject.
Ping?
These missed the libvirt-glib release, it would be nice to get them in... :)
Sorry I had missed them somehow. ACK series. -- Regards, Zeeshan Ali (Khattak) ________________________________________ Befriend GNOME: http://www.gnome.org/friends/

On Wed, Jul 22, 2015 at 01:27:45PM +0100, Zeeshan Ali (Khattak) wrote:
On Wed, Jul 22, 2015 at 11:41 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Hey,
On Fri, Jul 17, 2015 at 09:01:46AM +0200, Christophe Fergeau wrote:
On Thu, Jul 09, 2015 at 10:13:16AM +0200, Christophe Fergeau wrote:
Hey,
This patch series makes tests/test-gconfig valgrind-clean, and refactors two setters in GVirConfigDomainVideo to make them use the helpers provided by GVirConfigObject.
Ping?
These missed the libvirt-glib release, it would be nice to get them in... :)
Sorry I had missed them somehow. ACK series.
Thanks, pushed now (along with a 5th one which I pushed by mistake /o\ , sent to the list separately ) Christophe
participants (2)
-
Christophe Fergeau
-
Zeeshan Ali (Khattak)