Re: [Xen-devel] [PATCH 2/4] xen/arm: initialize access

2018-10-15 Thread Julien Grall

Hi,

Please use scripts/get_maintainers.pl to CC relevant maintainers. In 
this case you need to add Ravzan and Tamas.



On 15/10/2018 10:56, Stefano Stabellini wrote:

Initialize variable *access before returning it back to the caller.


Same as the previous patch, why do you need this?

Cheers,



Signed-off-by: Stefano Stabellini 
---
  xen/arch/arm/mem_access.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
index ba4ec78..10ab308 100644
--- a/xen/arch/arm/mem_access.c
+++ b/xen/arch/arm/mem_access.c
@@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
  };
  
  ASSERT(p2m_is_locked(p2m));

+*access = XENMEM_access_n;
  
  /* If no setting was ever set, just return rwx. */

  if ( !p2m->mem_access_enabled )



--
Julien Grall

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

Re: [Xen-devel] [PATCH 2/4] xen/arm: initialize access

2018-10-15 Thread Tamas K Lengyel
On Mon, Oct 15, 2018 at 3:57 AM Stefano Stabellini
 wrote:
>
> Initialize variable *access before returning it back to the caller.
>
> Signed-off-by: Stefano Stabellini 
> ---
>  xen/arch/arm/mem_access.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
> index ba4ec78..10ab308 100644
> --- a/xen/arch/arm/mem_access.c
> +++ b/xen/arch/arm/mem_access.c
> @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
>  };
>
>  ASSERT(p2m_is_locked(p2m));
> +*access = XENMEM_access_n;

 Why XENMEM_access_n and why set this at all here?

Tamas

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

Re: [Xen-devel] [PATCH 2/4] xen/arm: initialize access

2018-10-15 Thread Stefano Stabellini
On Mon, 15 Oct 2018, Tamas K Lengyel wrote:
> On Mon, Oct 15, 2018 at 3:57 AM Stefano Stabellini
>  wrote:
> >
> > Initialize variable *access before returning it back to the caller.
> >
> > Signed-off-by: Stefano Stabellini 
> > ---
> >  xen/arch/arm/mem_access.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
> > index ba4ec78..10ab308 100644
> > --- a/xen/arch/arm/mem_access.c
> > +++ b/xen/arch/arm/mem_access.c
> > @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t 
> > gfn,
> >  };
> >
> >  ASSERT(p2m_is_locked(p2m));
> > +*access = XENMEM_access_n;
> 
>  Why XENMEM_access_n and why set this at all here?

Hi Tamas,

Yes, I missed an explanation. Initializing variables before passing them
as parameter or as a return value to a function is a safety
certification requirement. Also, it makes the code a bit nicer.

In the specific case of this function, *access is initialized before
returning in all cases but the return -ESRCH case. I thought the nicer
way to make sure *access is always initialized, making the code more
robust, would be to initialize *access at the beginning of the function
with a restrictive value.

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

Re: [Xen-devel] [PATCH 2/4] xen/arm: initialize access

2018-10-16 Thread Tamas K Lengyel
On Mon, Oct 15, 2018 at 7:14 PM Stefano Stabellini
 wrote:
>
> On Mon, 15 Oct 2018, Tamas K Lengyel wrote:
> > On Mon, Oct 15, 2018 at 3:57 AM Stefano Stabellini
> >  wrote:
> > >
> > > Initialize variable *access before returning it back to the caller.
> > >
> > > Signed-off-by: Stefano Stabellini 
> > > ---
> > >  xen/arch/arm/mem_access.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
> > > index ba4ec78..10ab308 100644
> > > --- a/xen/arch/arm/mem_access.c
> > > +++ b/xen/arch/arm/mem_access.c
> > > @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t 
> > > gfn,
> > >  };
> > >
> > >  ASSERT(p2m_is_locked(p2m));
> > > +*access = XENMEM_access_n;
> >
> >  Why XENMEM_access_n and why set this at all here?
>
> Hi Tamas,
>
> Yes, I missed an explanation. Initializing variables before passing them
> as parameter or as a return value to a function is a safety
> certification requirement. Also, it makes the code a bit nicer.
>
> In the specific case of this function, *access is initialized before
> returning in all cases but the return -ESRCH case. I thought the nicer
> way to make sure *access is always initialized, making the code more
> robust, would be to initialize *access at the beginning of the function
> with a restrictive value.

Got it, thanks, Please use p2m->default_access for this instead to be
consistent with similar code at other spots.

Tamas

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

Re: [Xen-devel] [PATCH 2/4] xen/arm: initialize access

2018-10-17 Thread Stefano Stabellini
On Tue, 16 Oct 2018, Tamas K Lengyel wrote:
> On Mon, Oct 15, 2018 at 7:14 PM Stefano Stabellini
>  wrote:
> >
> > On Mon, 15 Oct 2018, Tamas K Lengyel wrote:
> > > On Mon, Oct 15, 2018 at 3:57 AM Stefano Stabellini
> > >  wrote:
> > > >
> > > > Initialize variable *access before returning it back to the caller.
> > > >
> > > > Signed-off-by: Stefano Stabellini 
> > > > ---
> > > >  xen/arch/arm/mem_access.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
> > > > index ba4ec78..10ab308 100644
> > > > --- a/xen/arch/arm/mem_access.c
> > > > +++ b/xen/arch/arm/mem_access.c
> > > > @@ -47,6 +47,7 @@ static int __p2m_get_mem_access(struct domain *d, 
> > > > gfn_t gfn,
> > > >  };
> > > >
> > > >  ASSERT(p2m_is_locked(p2m));
> > > > +*access = XENMEM_access_n;
> > >
> > >  Why XENMEM_access_n and why set this at all here?
> >
> > Hi Tamas,
> >
> > Yes, I missed an explanation. Initializing variables before passing them
> > as parameter or as a return value to a function is a safety
> > certification requirement. Also, it makes the code a bit nicer.
> >
> > In the specific case of this function, *access is initialized before
> > returning in all cases but the return -ESRCH case. I thought the nicer
> > way to make sure *access is always initialized, making the code more
> > robust, would be to initialize *access at the beginning of the function
> > with a restrictive value.
> 
> Got it, thanks, Please use p2m->default_access for this instead to be
> consistent with similar code at other spots.

OK, no prob

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