Re: [Xen-devel] [PATCH 1/4] libxl: CODING_STYLE: Much new material

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [PATCH 1/4] libxl: CODING_STYLE: Much new material"):
> On Wed, 2014-11-05 at 14:43 +0000, Ian Jackson wrote:
> > +The following local variable names should be used where applicable:
> > +
> > +  int rc;    /* a libxl error code - and not anything else */
> > +  int r;     /* the return value from a system call (or libxc call) */
> 
> Quite a bit more "ret" for this one. Probably quite a few are being
> misused as rc too, which is perhaps why you omitted it?

I have avoided specifying `ret' here because ret is used
indiscriminately for pretty much any int returned from a function.

> > +  * If the function is to return a libxl error value, `rc' is
> > +    used to contain the error codem, but it is NOT initialised:
> 
> I suspect this is a typo? (but then I never studied latin...)

Fixed.

> > +  * Function calls which might fail (ie most function calls) are
> > +    handled by putting the return/status value into a variable, and
> > +    then checking it in a separate statement:
> > +            evg->vdev = strdup(vdev);
> > +            if (!evg->vdev) { rc = ERROR_NOMEM; goto out; }
> 
> A slightly dodgy example because this should be GCSTRDUP(NOGC, vdev) and
> therefore can't fail ;-)

Example replaced with:
    char *dompath = libxl__xs_get_dompath(gc, bl->domid);
    if (!dompath) { rc = ERROR_FAIL; goto out; }

> > +Nontrivial data structures (in structs) should come with an idempotent
> > +_destroy function, which must free all resources associated with the
> 
> _dispose.

Fixed (twice).

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel