[libvirt] [libvirt-glib] gobject: Correct docs for gvir_storage_pool_get_info()
The returned GVirStoragePoolInfo pointer is not a GObject so it must not be unrefed using g_object_unref(). Since gvir_storage_pool_info_free() is private function, callers must either use g_slice_free() or g_boxed_free(). --- Perhaps we should just make gvir_storage_pool_info_free() public instead? libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 7f26b1b..f015efa 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -277,8 +277,8 @@ GVirConfigStoragePool *gvir_storage_pool_get_config(GVirStoragePool *pool, * @pool: the storage_pool * @err: Place-holder for possible errors * - * Returns: (transfer full): the info. The returned object should be - * unreffed with g_object_unref() when no longer needed. + * Returns: (transfer full): the info. The returned pointer should be + * freed using either #g_slice_free() or #g_boxed_free() when no longer needed. */ GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool, GError **err) -- 2.4.3
On Wed, Sep 23, 2015 at 01:44:28PM +0100, Zeeshan Ali (Khattak) wrote:
The returned GVirStoragePoolInfo pointer is not a GObject so it must not be unrefed using g_object_unref(). Since gvir_storage_pool_info_free() is private function, callers must either use g_slice_free() or g_boxed_free(). ---
Perhaps we should just make gvir_storage_pool_info_free() public instead?
libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 7f26b1b..f015efa 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -277,8 +277,8 @@ GVirConfigStoragePool *gvir_storage_pool_get_config(GVirStoragePool *pool, * @pool: the storage_pool * @err: Place-holder for possible errors * - * Returns: (transfer full): the info. The returned object should be - * unreffed with g_object_unref() when no longer needed. + * Returns: (transfer full): the info. The returned pointer should be + * freed using either #g_slice_free() or #g_boxed_free() when no longer needed. */ GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool, GError **err)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, Sep 23, 2015 at 2:18 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Wed, Sep 23, 2015 at 01:44:28PM +0100, Zeeshan Ali (Khattak) wrote:
The returned GVirStoragePoolInfo pointer is not a GObject so it must not be unrefed using g_object_unref(). Since gvir_storage_pool_info_free() is private function, callers must either use g_slice_free() or g_boxed_free(). ---
Perhaps we should just make gvir_storage_pool_info_free() public instead?
libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 7f26b1b..f015efa 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -277,8 +277,8 @@ GVirConfigStoragePool *gvir_storage_pool_get_config(GVirStoragePool *pool, * @pool: the storage_pool * @err: Place-holder for possible errors * - * Returns: (transfer full): the info. The returned object should be - * unreffed with g_object_unref() when no longer needed. + * Returns: (transfer full): the info. The returned pointer should be + * freed using either #g_slice_free() or #g_boxed_free() when no longer needed. */ GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool, GError **err)
ACK
Thanks but just to be sure, I hope you didn't miss the details and this comment below it:
Perhaps we should just make gvir_storage_pool_info_free() public instead?
-- Regards, Zeeshan Ali (Khattak) ________________________________________ Befriend GNOME: http://www.gnome.org/friends/
On Wed, Sep 23, 2015 at 02:54:51PM +0100, Zeeshan Ali (Khattak) wrote:
On Wed, Sep 23, 2015 at 2:18 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Wed, Sep 23, 2015 at 01:44:28PM +0100, Zeeshan Ali (Khattak) wrote:
The returned GVirStoragePoolInfo pointer is not a GObject so it must not be unrefed using g_object_unref(). Since gvir_storage_pool_info_free() is private function, callers must either use g_slice_free() or g_boxed_free(). ---
Perhaps we should just make gvir_storage_pool_info_free() public instead?
libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 7f26b1b..f015efa 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -277,8 +277,8 @@ GVirConfigStoragePool *gvir_storage_pool_get_config(GVirStoragePool *pool, * @pool: the storage_pool * @err: Place-holder for possible errors * - * Returns: (transfer full): the info. The returned object should be - * unreffed with g_object_unref() when no longer needed. + * Returns: (transfer full): the info. The returned pointer should be + * freed using either #g_slice_free() or #g_boxed_free() when no longer needed. */ GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool, GError **err)
ACK
Thanks but just to be sure, I hope you didn't miss the details and this comment below it:
Perhaps we should just make gvir_storage_pool_info_free() public instead?
Since we've defined it as a boxed type, I think recommending g_boxed_free is the right solution. We should make sure our docs actually tell people this is a boxed type if they don't already :-) We should *not* in fact recommend g_slice_free(), as the use of the slice allocator is an internal implementation detail. We should be free to change to use a different allocator without breaking clients. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, Sep 23, 2015 at 3:00 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Wed, Sep 23, 2015 at 02:54:51PM +0100, Zeeshan Ali (Khattak) wrote:
On Wed, Sep 23, 2015 at 2:18 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Wed, Sep 23, 2015 at 01:44:28PM +0100, Zeeshan Ali (Khattak) wrote:
The returned GVirStoragePoolInfo pointer is not a GObject so it must not be unrefed using g_object_unref(). Since gvir_storage_pool_info_free() is private function, callers must either use g_slice_free() or g_boxed_free(). ---
Perhaps we should just make gvir_storage_pool_info_free() public instead?
libvirt-gobject/libvirt-gobject-storage-pool.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 7f26b1b..f015efa 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -277,8 +277,8 @@ GVirConfigStoragePool *gvir_storage_pool_get_config(GVirStoragePool *pool, * @pool: the storage_pool * @err: Place-holder for possible errors * - * Returns: (transfer full): the info. The returned object should be - * unreffed with g_object_unref() when no longer needed. + * Returns: (transfer full): the info. The returned pointer should be + * freed using either #g_slice_free() or #g_boxed_free() when no longer needed. */ GVirStoragePoolInfo *gvir_storage_pool_get_info(GVirStoragePool *pool, GError **err)
ACK
Thanks but just to be sure, I hope you didn't miss the details and this comment below it:
Perhaps we should just make gvir_storage_pool_info_free() public instead?
Since we've defined it as a boxed type, I think recommending g_boxed_free is the right solution.
It's just that it's slightly awkward to use that, since you need to pass it the gtype, not just the pointer.
We should make sure our docs actually tell people this is a boxed type if they don't already :-)
We should *not* in fact recommend g_slice_free(), as the use of the slice allocator is an internal implementation detail. We should be free to change to use a different allocator without breaking clients.
Sure, I pushed with g_slice_free() mention removed. -- Regards, Zeeshan Ali (Khattak) ________________________________________ Befriend GNOME: http://www.gnome.org/friends/
participants (2)
-
Daniel P. Berrange -
Zeeshan Ali (Khattak)