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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Nov 05, 2014 at 02:43:47PM +0000, Ian Jackson wrote:
[...]
> +  * 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; }
> +
> +  * If a resource is freed in the main body of the function (for
> +    example, in a loop), the corresponding variable has to be reset to
> +    the sentinel at the point where it's freed.
> +

However useful this one is, in practice this looks very tedious to
enforce and easy to slip under reviewer's eyes.

> +Whether to use the `out' path for successful returns as well as error
> +returns is a matter of taste and convenience for the specific
> +function.  Not reusing the out path is fine if the duplicated function
> +exit code is only `CTX_UNLOCK; GC_FREE;' (or similar).
> +
> +If you reuse the `out' path for successful returns, there may be
> +resources which are to be returned to the caller rather than freed.
> +In that case you have to reset the local variable to `nothing here',
> +to avoid the resource being freed on the out path.  That resetting
> +should be done immediately after the resource value is stored at the
> +applicable _r function parameter (or equivalent).  Do not test `rc' in
> +the out section, to discover whether to free things.
> +
> +The uses of the single-line formatting in the examples above are
> +permitted exceptions to the usual libxl code formatting rules.
> +
> +
> +
> +IDEMPOTENT DATA STRUCTURE CONSTRUCTION/DESTRUCTION
> +--------------------------------------------------
> +
> +Nontrivial data structures (in structs) should come with an idempotent
> +_destroy function, which must free all resources associated with the

_dispose?

> +data structure (but not free the struct itself).
> +
> +Such a struct should also come with an _init function which
> +initialises the struct so that _destroy is a no-op.
> +

Is it worth mentioning that IDL-defined structures already have _dispose
and _init autogenerated, and _dispose is not idempotent at the moment?
In fact, the generated _dispose function explicitly poisons the
structure.

Wei.

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