This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [Qemu-devel] [PATCH 02/10] xen: backend driver core

To: Anthony Liguori <aliguori@xxxxxxxxxx>
Subject: [Xen-devel] Re: [Qemu-devel] [PATCH 02/10] xen: backend driver core
From: Gerd Hoffmann <kraxel@xxxxxxxxxx>
Date: Wed, 08 Apr 2009 11:21:59 +0200
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, qemu-devel@xxxxxxxxxx
Delivery-date: Wed, 08 Apr 2009 02:22:27 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <49DBAC54.7010500@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1239115497-1884-1-git-send-email-kraxel@xxxxxxxxxx> <1239115497-1884-3-git-send-email-kraxel@xxxxxxxxxx> <49DBAC54.7010500@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b3pre) Gecko/20090324 Fedora/3.0-2.1.beta2.fc11 Thunderbird/3.0b2
On 04/07/09 21:41, Anthony Liguori wrote:
+/* private */
+static TAILQ_HEAD(XenDeviceHead, XenDevice) xendevs =
+static int debug = 0;

Would be better to have all of this in a structure that had a single
static instance. Would be even better if you could avoid requiring the
static instance.

Huh?  Point being?  This is just a list head, i.e. a pointer (or two?).

+char *xenstore_read_str(const char *base, const char *node)
+ char abspath[XEN_BUFSIZE];
+ unsigned int len;
+ snprintf(abspath, sizeof(abspath), "%s/%s", base, node);
+ return xs_read(xenstore, 0, abspath, &len);

xs_read() is a xenstore API, it's returning malloc()'d memory.

+int xenstore_read_int(const char *base, const char *node, int *ival)
+ char *val;
+ int rc = -1;
+ val = xenstore_read_str(base, node);
+ if (val && 1 == sscanf(val, "%d", ival))
+ rc = 0;
+ qemu_free(val);

And here you're free()'ing with qemu_free.

Oops.  Good catch.

+ xendev = qemu_mallocz(ops->size);
+ if (!xendev)
+ return NULL;

No need to check malloc failures.

Will fix.

+ dev = xs_directory(xenstore, 0, path, &cdev);
+ qemu_free(dev);

Mixing qemu_free() with malloc'd memory.

This too.

You also have open coded calls to fprintf(stderr)
whereas you've introduced a higher level function.

The high-level function wants a device instance and thus doesn't work everythere. Nevertheless the code probably should use the new qemu_log() function instead. I'll look into this.


Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>