Re: [Xen-devel] [PATCH 4/8] xen/arm: Add support for read-only foreign mappings

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 11/15/18 1:19 PM, Andrii Anisov wrote:
> > So I would prefer to stick with _t which is quite common within the p2m
> > code base so far.
> I've found a similar code only in one place - p2m_get_entry()
> function. And it is, at least, somehow commented there:
> ...
>      /* Allow t to be NULL */
>      t = t ?: &_t;
>
>      *t = p2m_invalid;
> ...
And in x86 code...

> But IMO, it is really confusing to write a code to calculate and store
> a value which clearly is not needed by a caller.
I disagree, you directly know that t can be NULL. Although, I agree that 
a comment would help to understand.
>  From another hand, I'm not sure if a compiler would be intelligent
> enough to factor out the odd code from execution pass on the incoming
> null pointer.
You can't assume the compiler will inline even with the inline keyword.

> I'm sorry, but I can't pass my RB for `_t`.
I think this request is unreasonable because this is a matter of taste.

While this is a nice feature to have in Xen 4.12 and address a long 
standing open issue on Arm. I don't require it and I am not inclined to 
address bikeshedding.
So I will keep aside for now until someone cares about it.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel