[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 07/15] libxl: provide libxl__xs_*_checked and libxl__xs_transaction_*

Ian Campbell writes ("Re: [Xen-devel] [PATCH 07/15] libxl: provide 
libxl__xs_*_checked and libxl__xs_transaction_*"):
> > +int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t,
> > +                           const char *path, const char **result_out)
> > +{
> > +    char *result = libxl__xs_read(gc, t, path);
> > +    if (!result) {
> > +        if (errno != ENOENT) {
> Can't you combine these with && ?

Yes, but this matches better the way I think of it.  I can change it
if you like.

> > +int libxl__xs_transaction_commit(libxl__gc *gc, xs_transaction_t *t)
> > +{
> > +        LOGE(ERROR, "could not commit xenstore transacton");
>                                                   transaction


> It would be much more useful if this function took a "const char
> *what" (even just __func__ from the caller) and logged it here.

I think that this call can only fail due to general failure of
xenstore (or our xenstore handle).  So a `what' here wouldn't help
much.  The other helpers are more likely to fail (permissions, for
example) but they can log the path.

I played about a bit with adding more information supplied by the
callers.  Ultimately what I seem to end up with is something that's
rather too clumsy to use without helper macros which I don't think
will assist with clarity.


        if (got) {
            const char *got_ret;
            rc = XLXS_READ(t, lds->ret_path, &got_ret);
            if (rc) goto out;

which is all very well but hardly a model of clarity.

An alternative would be to replace `xs_transaction_t t' with a struct
that contains at least the file and function name and a `what', which
would be passed to the transaction start helper.

So I think I would prefer to keep things as they are.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.