Re: [Xen-devel] [PATCH 2/4] xen/arm: initialize access
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
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
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
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
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