xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
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

  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).