Re: [PATCH-for-4.9 v1 2/8] dm_op: convert HVMOP_*ioreq_server*

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 25 November 2016 07:06
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu
> <wei.liu2@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; xen-
> devel@lists.xenproject.org; Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Subject: Re: [Xen-devel] [PATCH-for-4.9 v1 2/8] dm_op: convert
> HVMOP_*ioreq_server*
> 
> >>> On 24.11.16 at 18:02, <JBeulich@suse.com> wrote:
> >>>> On 18.11.16 at 18:13, <paul.durrant@citrix.com> wrote:
> >> +    {
> >> +        struct xen_dm_op_get_ioreq_server_info *data =
> >> +            &op.u.get_ioreq_server_info;
> >> +
> >> +        rc = hvm_get_ioreq_server_info(d, data->id,
> >> +                                       &data->ioreq_pfn,
> >> +                                       &data->bufioreq_pfn,
> >> +                                       &data->bufioreq_port);
> >
> > Before the call you should check the __pad field to be zero
> > (presumably also elsewhere).
> 
> And please no double underscores at the beginning of those field
> names; preferably none at all (as field names may collide with file
> scope object-like macros).

Ok.

> 
> >>  struct xen_dm_op {
> >>      uint32_t op;
> >> +    union {
> >
> > Even if no current structure needs it, I think we should have a 32-bit
> > padding field ahead of the union right away, to cover (current or
> > future) uint64_aligned_t uses inside the union members.
> 
> Actually I did overlook that the few instances of uint64_aligned_t
> are in direct union members, not in fields referenced, so this isn't just
> a "should" really.
> 

Ok, I'll go through and check field alignments again.

  Paul

> Jan


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