> It's not clear to me why you're moving the open call about.
This is because the current version does not invoke xs_daemon_close(xsh) if the
asprintf is return.
> this function always seems to leak kvs[0]
kvs[1]?
I will try to make a new path according to your suggestion. Thanks!
Jun Zhu
Citrix Systems UK
________________________________________
From: Ian Jackson
Sent: 08 September 2010 11:31
To: Jun Zhu (Intern)
Cc: Ian Campbell; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: ð´: [Xen-devel] [PATCH] libxl: make libxl c ommunicate with
xenstored by socket or xenb us driver
Jun Zhu (Intern) writes ("ð´: [Xen-devel] [PATCH] libxl: make libxl c
ommunicate with xenstored by socket or xenb us driver"):
> libxl uses socket or xenbus dev to communicate with xenstored.
Thanks for this patch. I have a few comments:
> @@ -1387,10 +1385,8 @@
> {
> libxl_device_model_starting *starting = for_spawn;
> char *kvs[3];
> - int rc;
> struct xs_handle *xsh;
>
> - xsh = xs_daemon_open();
> /* we mustn't use the parent's handle in the child */
>
> kvs[0] = "image/device-model-pid";
> @@ -1398,10 +1394,11 @@
> return;
> kvs[2] = NULL;
>
> - rc = xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> - if (rc)
> - return;
> - xs_daemon_close(xsh);
> + xsh = libxl_xs_open();
> + if (!xsh)
> + fprintf(stderr, "cannot connect to xenstore");
> + xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> + libxl_xs_close(xsh);
> }
It's not clear to me why you're moving the open call about. Your
error handling is incorrect, as passing a null xsh to xs_writev will
cause a segfault. Also, while you're there you should probably fix
the memory leak: this function always seems to leak kvs[0].
> - xsh = xs_daemon_open();
> + xsh = libxl_xs_open();
> + if (!xsh)
> + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno,
> + "cannot connect to xenstore");
Logging is not sufficient; you need to set rc and "goto out", or
return an error code, too. This pattern occurs a number of times in
your patch.
Finally, your patch seems to have been truncated at some point.
patch(1) complains:
patch: **** malformed patch at line 172:
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home