Re: [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes

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

 



On Wed, Feb 29, 2012 at 3:49 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> On Wed, Feb 29, 2012 at 03:44:20PM +0200, Zeeshan Ali (Khattak) wrote:
>> On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
>> > On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
>> >> From: "Zeeshan Ali (Khattak)" <zeeshanak@xxxxxxxxx>
>> >>
>> >> ---
>> >>  libvirt-gconfig/libvirt-gconfig-domain-interface.c |   35 ++++++++++++++++++++
>> >>  libvirt-gconfig/libvirt-gconfig-domain-interface.h |    4 ++
>> >>  libvirt-gconfig/libvirt-gconfig.sym                |    4 ++
>> >>  3 files changed, 43 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> >> index 85cc194..61d35bd 100644
>> >> --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> >> +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c
>> >> @@ -96,6 +96,41 @@ void gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface
>> >>                                                      "model", "type", model);
>> >>  }
>> >>
>> >> +const char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface)
>> >
>> > Unless I'm missing something, this should not be const (caller needs to
>> > free the returned string).
>>
>> String getters usually do return const in the gobject world[1] as
>> opposed to object getters as one require allocation/de-allocation and
>> the other only requires incrementing/decrementing a counter. Also is
>> the fact that strings are readily usable so you can just make a call
>> to the getter and thats it but objects are not usually readily usable
>> in the sense that you have to pass it to to some other function to do
>> something with it. Note that I'm just making educated guesses here as
>> to why 'const' for strings makes more sense as to why this practice is
>> followed.
>
> Strings are const in the gobject world when they are owned by the object
> and not by the caller of the getter (ie the caller doesn't need to free
> them). They are non-const when the caller has to free them. In this case,
> the strings need to be freed by the caller, thus "const" should not be
> used.

Ah, I just realized why that is so:

static char *libxml_str_to_glib(xmlChar *str)
{
    char *g_str;

    if (str == NULL)
        return NULL;
    g_str = g_strdup((char *)str);
    xmlFree(str);

    return g_str;
}

This function is not needed as all you needed was to cast the 'xmlChar
*' to 'const gchar *' and return const from all users of this
function. Since we still are not API/ABI stable, I propose we change
this all over as there is no need to force apps to free strings all
the time and waste processor/memory on all these string
allocation/de-allocation.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[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]