Re: [Xen-devel] [PATCH V4 16/24] libxl: copy function for builtin types

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-05-01 at 13:58 +0100, Wei Liu wrote:
> These functions will be used in later patch to deep-copy a structure.

Please can you document this in libxl.h alongside _init and _dispose
etc. Can you do parse_json and gen_json too (I know the second isn't
your mess, it's mine, sorry!).
> diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
> index 7f27c67..aab9d0a 100644
> --- a/tools/libxl/libxl_cpuid.c
> +++ b/tools/libxl/libxl_cpuid.c
> @@ -462,6 +462,39 @@ int 
> libxl_cpuid_policy_list_length(libxl_cpuid_policy_list *l)
>      return i;
>  }
>  
> +void libxl_cpuid_policy_list_copy(libxl_ctx *ctx,
> +                                  libxl_cpuid_policy_list *dst,
> +                                  libxl_cpuid_policy_list *src)

Just picking on this one at random, should they return an int even if
all the current uses can only return success? Is there any conceivable
way for any of these functions to fail (other than enomem which is
handled already). e.g. ERROR_INVALID because the input is bogus perhaps.
For consistency if any one can fail I think they should all return an
error code.

> diff --git a/tools/libxl/libxl_uuid.h b/tools/libxl/libxl_uuid.h
> index 93c65a7..541f0f8 100644
> --- a/tools/libxl/libxl_uuid.h
> +++ b/tools/libxl/libxl_uuid.h
> @@ -53,10 +53,14 @@ typedef struct {
>  
>  #endif
>  
> +typedef struct libxl__ctx libxl_ctx;
> +
>  int libxl_uuid_is_nil(libxl_uuid *uuid);
>  void libxl_uuid_generate(libxl_uuid *uuid);
>  int libxl_uuid_from_string(libxl_uuid *uuid, const char *in);
>  void libxl_uuid_copy(libxl_uuid *dst, const libxl_uuid *src);
> +void libxl_uuid_copy_ctx(libxl_ctx *ctx, libxl_uuid *dst,
> +                         const libxl_uuid *src);

Hrm, this is rather unfortunate.

All of these only take a ctx so they can use NOGC. I wonder if a #define
INIT_NOGC which provides a suitable gc (with maxsize == -1) might be
nicer than this?

Ian? What do you think?



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