Re: [Xen-devel] [PATCH v5 for-4.9 1/4] hvm/dmop: Box dmop_bufs rather than passing two parameters around

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper
> Sent: 10 April 2017 10:29
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Xen-devel <xen-
> devel@xxxxxxxxxxxxx>
> Cc: Jennifer Herbert <jennifer.herbert@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Julien Grall <julien.grall@xxxxxxx>
> Subject: Re: [PATCH v5 for-4.9 1/4] hvm/dmop: Box dmop_bufs rather than
> passing two parameters around
> 
> On 10/04/17 10:04, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> >> Sent: 07 April 2017 20:36
> >> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> >> Cc: Jennifer Herbert <jennifer.herbert@xxxxxxxxxx>; Andrew Cooper
> >> <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich <JBeulich@xxxxxxxx>; Paul
> >> Durrant <Paul.Durrant@xxxxxxxxxx>; Julien Grall <julien.grall@xxxxxxx>
> >> Subject: [PATCH v5 for-4.9 1/4] hvm/dmop: Box dmop_bufs rather than
> >> passing two parameters around
> >>
> >> From: Jennifer Herbert <Jennifer.Herbert@xxxxxxxxxx>
> >>
> >> No functional change.
> >>
> > Why is this a good thing? Passing two parameters around allowed for them
> to be in registers. I preferred the code as it was before.
> 
> a) It will always be inlined, so registers aren't relevant.

Why? I see nothing forcing the compiler to make it so.

>  Even if
> they were, all values are available directly with the pointer as a base,
> so there is no reduction in expressiveness.  (i.e. the previous code
> only increases register pressure).
> b) passing multiple parameters like that is a recipe for mistakes, and
> in this case, mistakes mean security vulnerabilities.

Given the locality of the code I don't buy that as an argument unless you're 
going to assert that passing more than one parameter is always wrong.

> c) It is necessary to make legible code for the later changes in this
> series.
> 

From my reading I don't think it would have an overly negative effect on the 
subsequent patches if this patch were dropped.

  Paul

> ~Andrew

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