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

Re: [libvirt] [libvirt-glib] Add getter for GVirConfigDomainOS.arch



On Mon, May 21, 2012 at 7:14 PM, Daniel P. Berrange <berrange redhat com> wrote:
> On Mon, May 21, 2012 at 07:06:32PM +0300, Zeeshan Ali (Khattak) wrote:
>> On Mon, May 21, 2012 at 3:13 PM, Christophe Fergeau <cfergeau redhat com> wrote:
>> > On Sat, May 19, 2012 at 05:45:31AM +0300, Zeeshan Ali (Khattak) wrote:
>> >> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
>> >>
>> >> ---
>> >>  libvirt-gconfig/libvirt-gconfig-domain-os.c |    7 +++++++
>> >>  libvirt-gconfig/libvirt-gconfig-domain-os.h |    1 +
>> >>  libvirt-gconfig/libvirt-gconfig.sym         |    1 +
>> >>  3 files changed, 9 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-os.c b/libvirt-gconfig/libvirt-gconfig-domain-os.c
>> >> index 9a1648a..2e4a69a 100644
>> >> --- a/libvirt-gconfig/libvirt-gconfig-domain-os.c
>> >> +++ b/libvirt-gconfig/libvirt-gconfig-domain-os.c
>> >> @@ -275,6 +275,13 @@ GList *gvir_config_domain_os_get_boot_devices(GVirConfigDomainOs *os)
>> >>      return devices;
>> >>  }
>> >>
>> >> +const char *gvir_config_domain_os_get_arch(GVirConfigDomainOs *os)
>> >> +{
>> >
>> > g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_OS(os), NULL);
>> >
>> > ACK with this added. Is it fine to return NULL when no arch is specified,
>> > or would it be better to return something else?
>>
>> I'm not very sure about returing default values in general since thats
>> very much a policy decision and we are not giving any clue to the app
>> that we are making the decision for it. Just saying I'm not sure, not
>> that I actually have a strong opinion either way.
>
> Returning NULL is the right thing todo IMHO, otherwise as apps read &
> update the XML new attributes/elements will unneccessarily get added
> to the doc.

Good point but doesn't this also apply to all enum props as well?
Currently all enum getters return a default value. Maybe we should
have a _NONE value added to all those enums and return that when no
value is set? Don't know what to do with uint* though except returning
0 or one of the G_MAXUINT* depending on the getter in question? Having
said that, we don't have any getters from uint* yet AFAICT.

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124


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