On 20/07/23 17:39, Julien Grall wrote:
Hi,
The e-mail is getting quite long. Can you trim the unnecessary bits when
replying?
Ok.
On 20/07/2023 15:23, Nicola Vetrini wrote:
The problem is that _t may be uninitialized, hence assigning its
address to t could be problematic.
But the value is set right after. IOW, there is no read between. So
how is this prob
Another way to address this is to initialize _t to a bad value and
use this variable in the body, then assign to t based on the value
just before returning.
IHMO, neither solution are ideal. I think we should investigate
whether Eclair can be improved.
[...]
I'll see what can be done about it, I'll reply when I have an answer.
What about this:
- p2m_type_t _t;
+ p2m_type_t _t = p2m_invalid;
[...]
t = t ?: &_t;
- *t = p2m_invalid;
+ *t = _t;
The resulting code is still quite confusing. I am still not quite sure
why ECLAIR can't understand the construct. Apologies if this was already
said, but this thread is getting quite long with many different issues.
So it is a bit difficult to navigate (which is why I suggested to split
and have a commit message explaining the rationale for each).
Anyway, if we can't improve Eclair, then my preference would be the
following:
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index de32a2d638ba..05d65db01b0c 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -496,16 +496,13 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t
gfn,
lpae_t entry, *table;
int rc;
mfn_t mfn = INVALID_MFN;
- p2m_type_t _t;
DECLARE_OFFSETS(offsets, addr);
ASSERT(p2m_is_locked(p2m));
BUILD_BUG_ON(THIRD_MASK != PAGE_MASK);
- /* Allow t to be NULL */
- t = t ?: &_t;
-
- *t = p2m_invalid;
+ if ( t )
+ *t = p2m_invalid;
if ( valid )
*valid = false;
@@ -549,7 +546,8 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
if ( p2m_is_valid(entry) )
{
- *t = entry.p2m.type;
+ if ( t )
+ *t = entry.p2m.type;
if ( a )
*a = p2m_mem_access_radix_get(p2m, gfn);
Ok, I'll make a separate patch.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
