Hi,
On 20/04/2020 10:16, Jan Beulich wrote:
On 18.04.2020 12:23, Julien Grall wrote:
On 30/03/2020 08:52, Jan Beulich wrote:
On 28.03.2020 11:52, Julien Grall wrote:
On 26/03/2020 15:39, Jan Beulich wrote:
On 22.03.2020 17:14, jul...@xen.org wrote:
@@ -3116,24 +3116,24 @@ int vcpu_destroy_pagetables(struct vcpu *v)
/* Free that page if non-zero */
do {
- if ( mfn )
+ if ( !mfn_eq(mfn, _mfn(0)) )
I admit I'm not fully certain either, but at the first glance
if ( mfn_x(mfn) )
would seem more in line with the original code to me (and then
also elsewhere).
It is doing *exactly* the same things. The whole point of typesafe
is to use typesafe helper not open-coding test everywhere.
It is also easier to spot any use of MFN 0 within the code as you
know could grep "_mfn(0)".
Therefore I will insist to the code as-is.
What I insit on is that readability of the result of such changes be
also kept in mind. The mfn_eq() construct is (I think) clearly less
easy to read and recognize than the simpler alternative suggested.
If mfn_eq() is less clear, then where do you draw the line when the
macro should or not be used?
I'm afraid there may not be a clear line to draw until everything
got converted.
I am sorry but this doesn't add up. Here you say that we can't have a
clear line to draw until everything is converted but...
I do seem to recall though that, perhaps in a
different context, Andrew recently agreed with my view here (Andrew,
please correct me if I'm wrong). It being a fuzzy thing, I guess
maintainers get to judge ...
... here you say the maintainers get to decide when to use mfn_eq() (or
other typesafe construction). So basically, we would never be able to
fully convert the code and therefore never draw a line.
As I am trying to convert x86 to use typesafe, I would like a bit more
guidelines on your expectation for typesafe. Can you clarify it?
Cheers,
--
Julien Grall