Re: [Xen-devel] [PATCH v4 3/4] Introduce the Xen 9pfs transport header

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.03.17 at 23:04, <sstabellini@xxxxxxxxxx> wrote:
> On Tue, 28 Mar 2017, Jan Beulich wrote:
>> >>> Stefano Stabellini <sstabellini@xxxxxxxxxx> 03/27/17 10:54 PM >>>
>> >On Mon, 27 Mar 2017, Jan Beulich wrote:
>> >> >>> On 24.03.17 at 19:31, <sstabellini@xxxxxxxxxx> wrote:
>> >> > +/*
>> >> > + * See docs/misc/9pfs.markdown in xen.git for the full specification:
>> >> > + * https://xenbits.xen.org/docs/unstable/misc/9pfs.html 
>> >> > + */
>> >> > +#pragma pack(push)
>> >> > +#pragma pack(1)
>> >> > +struct xen_9pfs_header {
>> >> > +       uint32_t size;
>> >> > +       uint8_t id;
>> >> > +       uint16_t tag;
>> >> > +};
>> >> > +#pragma pack(pop)
>> >> 
>> >> There's no precedent to using pragmas in the public headers, and
>> >> these aren't C99-compliant.
>> >
>> >I'll remove pragma, together with the definition of struct
>> >xen_9pfs_header: this structure is already defined as part of the 9p
>> >protocol, and it is already mentioned in the Xen 9pfs transport spec as
>> >well. In fact, both QEMU and Linux already have it defined. I don't
>> >think we need it here.
>> 
>> That'll deal with the immediate issue here, but not with the more general
>> implied one: Why would you want to have misaligned fields in a protocol
>> definition?
> 
> Because this header is not actually part of the Xen trasport protocol,
> it is defined by the 9pfs specification. That's why QEMU already had it.
> I cannot do anything about that. I was only redefining it here for
> convenience, because reading the header is required to figure out how
> big is a request (or a response).

If the size is all you care about, perhaps

struct xen_9pfs_header {
        uint8_t size[4];
        uint8_t id[1];
        uint8_t tag[2];
};

would do? (This made me notice you use hard tabs, which you
shouldn't in the Xen tree.)

Jan


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