From: Vincent Hanquez <vincent.hanquez@eu.citrix.com>
To: Andre Przywara <andre.przywara@amd.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
Ian Jackson <Ian.Jackson@eu.citrix.com>,
Keir Fraser <Keir.Fraser@eu.citrix.com>,
Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
Subject: Re: [PATCH 3/4] libxl: add version_info function
Date: Mon, 19 Apr 2010 09:08:38 +0100 [thread overview]
Message-ID: <4BCC0F86.8090707@eu.citrix.com> (raw)
In-Reply-To: <4BCB791E.7000204@amd.com>
On 18/04/10 22:26, Andre Przywara wrote:
> Xen provides a xen_version hypercall to query the values of several
> interesting things (like hypervisor version, commandline used,
> actual changeset, etc.). Create a user-friendly and efficient
> wrapper around the libxc function to provide values for xl info output.
instead of this chain of if else, why don't you just init the structure
to NULL at the beginning of the call, and not do any else ?
also the query capability seems a bit too much; the info call is
probably unlikely called into a tight loop, and I expect the number
of call to this to be 1 per starting toolstack or 1 per vm (at worst).
from an API point of view this would make it simpler too.
last thing, is instead of using strdup, you should use
libxl_sprintf(ctx, "%s", string) so that you don't have to worry about
freeing memory at all, and you can just omit the free_version_info call
completely.
--
Vincent
next prev parent reply other threads:[~2010-04-19 8:08 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-18 21:17 [PATCH 0/4] Add "xl info" command Andre Przywara
2010-04-18 21:23 ` [PATCH 1/4] libxl: extend physinfo structure Andre Przywara
2010-04-19 7:50 ` Vincent Hanquez
2010-04-19 15:27 ` [PATCH 1/4] libxl: extend physinfo structure [and 1 more messages] Ian Jackson
2010-04-18 21:25 ` [PATCH 2/4] libxl: add sched_get_id function Andre Przywara
2010-04-19 7:50 ` Vincent Hanquez
2010-04-19 15:29 ` [PATCH 2/4] libxl: add sched_get_id function [and 1 more messages] Ian Jackson
2010-04-18 21:26 ` [PATCH 3/4] libxl: add version_info function Andre Przywara
2010-04-19 8:08 ` Vincent Hanquez [this message]
2010-04-19 15:36 ` [PATCH 3/4] libxl: add version_info function [and 1 more messages] Ian Jackson
2010-04-19 16:07 ` Vincent Hanquez
2010-04-19 16:21 ` Ian Jackson
2010-04-19 16:41 ` Vincent Hanquez
2010-04-19 20:43 ` Andre Przywara
2010-04-20 8:36 ` Vincent Hanquez
2010-04-21 12:10 ` Andre Przywara
2010-04-21 12:53 ` Vincent Hanquez
2010-04-18 21:28 ` [PATCH 4/4] xl: add "xl info" command Andre Przywara
2010-04-19 15:38 ` Ian Jackson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4BCC0F86.8090707@eu.citrix.com \
--to=vincent.hanquez@eu.citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=Keir.Fraser@eu.citrix.com \
--cc=Stefano.Stabellini@eu.citrix.com \
--cc=andre.przywara@amd.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).