Jun Zhu (Intern) writes ("RE: [Xen-devel] [PATCH] libxl: make libxl communicate
with xenstored by socket or xenbus driver"):
> I make another one.
Thanks. (It helps if you say "v2" in the Subject line.)
> - rc = xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> - if (rc)
> - return;
> - xs_daemon_close(xsh);
> + /* we mustn't use the parent's handle in the child */
> + xsh = libxl_xs_open();
This, and some of the other parts of your patch, have what seem to be
unintended whitespace and formatting changes. Can you please make
sure that:
* Your indents are exactly 4 spaces
* You don't put any tabs in (your MUA may mangle them, and anyway
we don't like tabs)
* Curly backets (aka braces) should go on the same line as the
if and else.
> + if (!xsh)
> + {
> + fprintf(stderr, "cannot connect to xenstore");
> + free(kvs[1]);
> + return;
> + }
> + xs_writev(xsh, XBT_NULL, starting->dom_path, kvs);
> + libxl_xs_close(xsh);
> +
> + free(kvs[1]);
This would be better written with the "goto out" idiom, I think ?
> - ctx->xsh = xs_daemon_open();
> - if (!ctx->xsh) return ERROR_FAIL;
> - return 0;
> + ctx->xsh = libxl_xs_open();
> + if (!ctx->xsh)
> + {
> + XL_LOG_ERRNOVAL(ctx, XL_LOG_ERROR, errno,
> + "cannot connect to xenstore");
This logging, if it is appropriate, should surely be done in
libxl_xs_open, rather than coded out in each call ?
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home