Re: [Xen-devel] [PATCH v4 3/9] tools/libxc: Stream specification and some common code

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-04-30 at 19:36 +0100, Andrew Cooper wrote:
> diff --git a/tools/libxc/saverestore/stream_format.h 
> b/tools/libxc/saverestore/stream_format.h
> new file mode 100644
> index 0000000..efcca60
> --- /dev/null
> +++ b/tools/libxc/saverestore/stream_format.h

Reference to the spec (which doesn't appear to be in this series
anywhere BTW)

> @@ -0,0 +1,158 @@
> +#ifndef __STREAM_FORMAT__H
> +#define __STREAM_FORMAT__H
> +
> +#include <inttypes.h>
> +
> +/* TODO - find somewhere more appropriate. rec_tsc_info needs it for 64bit */

You seem to use it on all of them though. Can we not use explicit
padding, as you seem to do for many of the other structs?

> +#ifndef __packed
> +#define __packed __attribute__((packed))
> +#endif
> +
> +/*
> + * Image Header
> + */
> +struct __packed ihdr

So long as this remains confined to libxc/saverestore/*.[ch] we can
avoid namespacing/prefixing. I suppose that is a safe assumption.

> +{
> +    uint64_t marker;
> +    uint32_t id;
> +    uint32_t version;
> +    uint16_t options;
> +    uint16_t _res1;
> +    uint32_t _res2;
> +};
> +
> +#define IHDR_MARKER  0xffffffffffffffffULL
> +#define IHDR_ID      0x58454E46U
> +#define IHDR_VERSION 1
> +
> +#define _IHDR_OPT_ENDIAN 0
> +#define IHDR_OPT_LITTLE_ENDIAN (0 << _IHDR_OPT_ENDIAN)
> +#define IHDR_OPT_BIG_ENDIAN    (1 << _IHDR_OPT_ENDIAN)
> +
> +/*
> + * Domain Header
> + */
> +struct __packed dhdr
> +{
> +    uint32_t type;
> +    uint16_t page_shift;
> +    uint16_t _res1;
> +    uint32_t xen_major;
> +    uint32_t xen_minor;
> +};
> +
> +#define DHDR_TYPE_x86_pv  0x00000001U
> +#define DHDR_TYPE_x86_hvm 0x00000002U
> +#define DHDR_TYPE_x86_pvh 0x00000003U
> +#define DHDR_TYPE_arm     0x00000004U

That capitalisation here (and elsewhere) seems rather unconventional. 

BTW IHDR_OPT is also inconsistent with the pattern you've established.
TBH I'd prefer more conventional upper case #defines. I suppose you want
to avoid enums for the ABI-confusing properties they have.



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