[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] xen_console: support the new extended xenstore
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH] xen_console: support the new extended xenstore protocol |
Date: |
Tue, 28 Jun 2011 15:21:22 +0100 |
User-agent: |
Alpine 2.00 (DEB 1167 2008-08-23) |
On Tue, 28 Jun 2011, Peter Maydell wrote:
> On 28 June 2011 12:34, <address@hidden> wrote:
> > + path = xs_get_domain_path(xs, domid);
> > + if (path == NULL) {
> > + fprintf(stderr, "xs_get_domain_path() error\n");
> > + return -1;
> > + }
>
> Don't we need to call xs_daemon_close() on these error-exit paths?
>
> > + if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) {
> > + fprintf(stderr, "xs_write for '%s' fail", string);
> > + return -1;
> > + }
>
> ...and this one's leaking the path string too.
>
> > +void xenstore_store_pv_console_info(int i, CharDriverState *chr)
> > +{
> > + char buf[32];
> > +
> > + if (i == 0) {
> > + snprintf(buf, sizeof(buf), "/console");
> > + store_dev_info(xen_domid, chr, buf);
>
> Am I missing something, or could you just pass "/console" straight
> to store_dev_info() without bothering to copy it into buf[] here?
All very good points as usual.
I'll send another patch with the error paths fixed and the useless
snprintf removed.