Re: [Xen-devel] [PATCH 1/4] hvm/dmop: Box dmop_args rather than passing multiple parameters around

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.04.17 at 10:29, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 21/04/2017 09:10, Paul Durrant wrote:
>>> -----Original Message-----
>>> From: Andrew Cooper [mailto:amc96@xxxxxxxxxxxxxxxx] On Behalf Of
>>> Andrew Cooper
>>> Sent: 21 April 2017 09:04
>>> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Jennifer Herbert
>>> <jennifer.herbert@xxxxxxxxxx>; Xen-devel <xen-devel@xxxxxxxxxxxxx>
>>> Cc: Jan Beulich <JBeulich@xxxxxxxx>; Julien Grall <julien.grall@xxxxxxx>
>>> Subject: Re: [PATCH 1/4] hvm/dmop: Box dmop_args rather than passing
>>> multiple parameters around
>>>
>>> On 21/04/2017 08:54, Paul Durrant wrote:
>>>>> -----Original Message-----
>>>>> From: jennifer.herbert@xxxxxxxxxx [mailto:jennifer.herbert@xxxxxxxxxx]
>>>>> Sent: 20 April 2017 19:00
>>>>> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
>>>>> Cc: Jennifer Herbert <jennifer.herbert@xxxxxxxxxx>; Andrew Cooper
>>>>> <Andrew.Cooper3@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>;
>>>>> Jan Beulich <JBeulich@xxxxxxxx>; Julien Grall <julien.grall@xxxxxxx>
>>>>> Subject: [PATCH 1/4] hvm/dmop: Box dmop_args rather than passing
>>>>> multiple parameters around
>>>>>
>>>>> From: Jennifer Herbert <Jennifer.Herbert@xxxxxxxxxx>
>>>>>
>>>>> No functional change.
>>>>>
>>>>> Signed-off-by: Jennifer Herbert <Jennifer.Herbert@xxxxxxxxxx>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>>> --
>>>>> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
>>>>> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>>>> CC: Julien Grall <julien.grall@xxxxxxx>
>>>>> ---
>>>>>  xen/arch/x86/hvm/dm.c | 47 ++++++++++++++++++++++++++++--------
>>> ----
>>>>> -------
>>>>>  1 file changed, 28 insertions(+), 19 deletions(-)
>>>>>
>>>>> diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
>>>>> index d72b7bd..fb4bcec 100644
>>>>> --- a/xen/arch/x86/hvm/dm.c
>>>>> +++ b/xen/arch/x86/hvm/dm.c
>>>>> @@ -25,6 +25,13 @@
>>>>>
>>>>>  #include <xsm/xsm.h>
>>>>>
>>>>> +struct dmop_args {
>>>>> +    domid_t domid;
>>>>> +    unsigned int nr_bufs;
>>>>> +    /* Reserve enough buf elements for all current hypercalls. */
>>>>> +    struct xen_dm_op_buf buf[2];
>>>>> +};
>>>>> +
>>>>>  static bool copy_buf_from_guest(const xen_dm_op_buf_t bufs[],
>>>>>                                  unsigned int nr_bufs, void *dst,
>>>>>                                  unsigned int idx, size_t dst_size)
>>>>> @@ -287,16 +294,14 @@ static int inject_event(struct domain *d,
>>>>>      return 0;
>>>>>  }
>>>>>
>>>>> -static int dm_op(domid_t domid,
>>>>> -                 unsigned int nr_bufs,
>>>>> -                 xen_dm_op_buf_t bufs[])
>>>>> +static int dm_op(struct dmop_args *op_args)
>>>> Shouldn't this be a const pointer?
>>> No.  copy_to_guest_buf() uses a non const reference of op_args-
>>>> buf[$IDX].
>> Can't that be const too (as I commented in the relevant patch)?
> 
> No.  That is not legal in the C typesystem.
> 
> copy_to_guest_offset(args->buf[buf_idx].h, ...) really really uses a non
> constant .h here.
> 
> The broken quirk of the of the C typesystem which loses const when
> following pointers doesn't apply here, because buf[] is an embedded
> array and properly inherits the constness of the args pointer.

But the type of what .h refers to isn't affected by this - it'll remain
a handle of void, the const-ness doesn't propagate there. After
all this is just a pointer equivalent, i.e.

struct xen_dm_op_buf {
    void *h;
    ...
};

The const Paul suggests to apply would change this to
"void *const" rather than "const void *", and that should be fine.

Jan


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