[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-4.15] xen/mm: Fix build when CONFIG_HVM=n and CONFIG_COVERAGE=y [and 1 more messages]



Jan Beulich writes ("Re: [PATCH for-4.15] xen/mm: Fix build when CONFIG_HVM=n 
and CONFIG_COVERAGE=y [and 1 more messages]"):
> On 01.02.2021 15:54, Ian Jackson wrote:
> > Julien Grall writes ("[PATCH for-4.15] xen/mm: Fix build when CONFIG_HVM=n 
> > and CONFIG_COVERAGE=y"):
...
> > Jan, can you confirm whether in your opinion this patch as originally
> > posted by Julien is *correct* as is ?  In particular, Julien did not
> > intend a functional change.  Have you satisfied yourself that there is
> > no functional change here ?
> 
> Yes and yes.
> 
> > I understand your objectiion above to relate to style or neatness,
> > rather than function.  Is that correct ?
> 
> Yes.

Right, thanks.

> >  And that your proposed
> > additional change would have some impact whilch would have to be
> > assessed.
> 
> The first of the proposed alternatives may need further
> investigation, yes. The second of the alternatives would
> shrink this patch to a 2-line one, i.e. far less code
> churn, and is not in need of any assessment afaia. In
> fact I believe this latter alternative was discussed as
> the approach to take here, before the patch was submitted.

Sorry I missed that part.  I would be happy with that other approach
too, so for that approach (adding a duplicated ASSERT) is also

Release-Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>

I'm not a huge fan of code duplication, in general.  I suggest that if
the ASSERT is duplicated it might be worth leaving comment(s) by each
one pointing to the other.

Ian.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.