> > + return -1; > > + } > > + > > + VmaHeader *h = (VmaHeader *)vmar->head_data; > > + > > + if (h->magic != VMA_MAGIC) { > > + error_setg(errp, "not a vma file - wrong magic number"); > > + return -1; > > + } > > Doesn't seem like this is endian-safe. h->magic is host-endian, but VMA_MAGIC > is big-endian.
h->magic is read from the file, so it should be also big-endian. > > + > > +/* File Format Definitions */ > > + > > +#define VMA_MAGIC (GUINT32_TO_BE(('V'<<24)|('M'<<16)|('A'<<8)|0x00)) > > +#define VMA_EXTENT_MAGIC > > +(GUINT32_TO_BE(('V'<<24)|('M'<<16)|('A'<<8)|'E')) > > Do we care about EBCDIC, in which case you should be using raw hex values > instead of relying on ASCII conversion of your magic number? > That is, I'd much rather see: > > #define VMA_MAGIC 0x564D4100 /* ascii "VMA\0" */ The other code in qemu (for example block/qcow2.h) also use this style: #define QCOW_MAGIC (('Q' << 24) | ('F' << 16) | ('I' << 8) | 0xfb) > > + > > +typedef struct VmaDeviceInfoHeader { > > + uint32_t devname_ptr; /* offset into blob_buffer table */ > > + uint32_t reserved0; > > + uint64_t size; /* device size in bytes */ > > + uint64_t reserved1; > > + uint64_t reserved2; > > +} VmaDeviceInfoHeader; > > + > > +typedef struct VmaHeader { > > + uint32_t magic; > > + uint32_t version; > > + unsigned char uuid[16]; > > + int64_t ctime; > > + unsigned char md5sum[16]; > > + > > + uint32_t blob_buffer_offset; > > + uint32_t blob_buffer_size; > > + uint32_t header_size; > > + > > + unsigned char reserved[1984]; > > + > > + uint32_t config_names[VMA_MAX_CONFIGS]; /* offset into blob_buffer > table */ > > + uint32_t config_data[VMA_MAX_CONFIGS]; /* offset into > > + blob_buffer table */ > > + > > + VmaDeviceInfoHeader dev_info[256]; } VmaHeader; > > Is it worth a compile-time assertion that this header is a fixed size, to > make sure > that future edits evenly reduce the size of reserved when carving out new > fields? I tried to do that, but it seems that gcc does not support sizeof in preprocessor conditionals, so I added a runtime check in vma_writer_create(). Do you have an example how I can add a compile-time assertion that this header is a fixed size?