[19/24] errno may not be a gobal R/W variable, use a local variable instead (fix build on NetBSD)

Commit Message

Manuel Bouyer Dec. 14, 2020, 4:36 p.m. UTC

---
 tools/xenpaging/xenpaging.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Roger Pau Monne Dec. 29, 2020, 2:38 p.m. UTC | #1

On Mon, Dec 14, 2020 at 05:36:18PM +0100, Manuel Bouyer wrote:
> ---
>  tools/xenpaging/xenpaging.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> index 33098046c2..39c8c83b4b 100644
> --- a/tools/xenpaging/xenpaging.c
> +++ b/tools/xenpaging/xenpaging.c
> @@ -180,10 +180,11 @@ static int xenpaging_get_tot_pages(struct xenpaging *paging)
>  static void *init_page(void)
>  {
>      void *buffer;
> +    int rc;
>  
>      /* Allocated page memory */
> -    errno = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> -    if ( errno != 0 )
> +    rc = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> +    if ( rc != 0 )

I think the point of setting errno here is because posix_memalign
doesn't set it and instead returns an error code. The caller of
init_page uses PERROR in order to print the error which his expected to
be in errno.

I don't think this is the only place in Xen code that errno is set, why
are the others fine but not this instance?

Thanks, Roger.

Manuel Bouyer Jan. 4, 2021, 10:56 a.m. UTC | #2

On Tue, Dec 29, 2020 at 03:38:53PM +0100, Roger Pau Monné wrote:
> On Mon, Dec 14, 2020 at 05:36:18PM +0100, Manuel Bouyer wrote:
> > ---
> >  tools/xenpaging/xenpaging.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> > index 33098046c2..39c8c83b4b 100644
> > --- a/tools/xenpaging/xenpaging.c
> > +++ b/tools/xenpaging/xenpaging.c
> > @@ -180,10 +180,11 @@ static int xenpaging_get_tot_pages(struct xenpaging *paging)
> >  static void *init_page(void)
> >  {
> >      void *buffer;
> > +    int rc;
> >  
> >      /* Allocated page memory */
> > -    errno = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> > -    if ( errno != 0 )
> > +    rc = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> > +    if ( rc != 0 )
> 
> I think the point of setting errno here is because posix_memalign
> doesn't set it and instead returns an error code. The caller of
> init_page uses PERROR in order to print the error which his expected to
> be in errno.

I understand this. But on NetBSD, errno is:
#define errno (*__errno())

(I think this is related to thread-safety).

> 
> I don't think this is the only place in Xen code that errno is set, why
> are the others fine but not this instance?

probably this code is not used on NetBSD ?

Manuel Bouyer Jan. 4, 2021, 1:30 p.m. UTC | #3

On Mon, Jan 04, 2021 at 11:56:14AM +0100, Manuel Bouyer wrote:
> On Tue, Dec 29, 2020 at 03:38:53PM +0100, Roger Pau Monné wrote:
> > On Mon, Dec 14, 2020 at 05:36:18PM +0100, Manuel Bouyer wrote:
> > > ---
> > >  tools/xenpaging/xenpaging.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> > > index 33098046c2..39c8c83b4b 100644
> > > --- a/tools/xenpaging/xenpaging.c
> > > +++ b/tools/xenpaging/xenpaging.c
> > > @@ -180,10 +180,11 @@ static int xenpaging_get_tot_pages(struct xenpaging *paging)
> > >  static void *init_page(void)
> > >  {
> > >      void *buffer;
> > > +    int rc;
> > >  
> > >      /* Allocated page memory */
> > > -    errno = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> > > -    if ( errno != 0 )
> > > +    rc = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
> > > +    if ( rc != 0 )
> > 
> > I think the point of setting errno here is because posix_memalign
> > doesn't set it and instead returns an error code. The caller of
> > init_page uses PERROR in order to print the error which his expected to
> > be in errno.
> 
> I understand this. But on NetBSD, errno is:
> #define errno (*__errno())
> 
> (I think this is related to thread-safety).
> 
> > 
> > I don't think this is the only place in Xen code that errno is set, why
> > are the others fine but not this instance?
> 
> probably this code is not used on NetBSD ?

I was wrong, errno is writable but it needs #include <errno.h>
I reverted this a fixed it the right way

Patch

diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
index 33098046c2..39c8c83b4b 100644
--- a/tools/xenpaging/xenpaging.c
+++ b/tools/xenpaging/xenpaging.c
@@ -180,10 +180,11 @@  static int xenpaging_get_tot_pages(struct xenpaging *paging)
 static void *init_page(void)
 {
     void *buffer;
+    int rc;
 
     /* Allocated page memory */
-    errno = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
-    if ( errno != 0 )
+    rc = posix_memalign(&buffer, XC_PAGE_SIZE, XC_PAGE_SIZE);
+    if ( rc != 0 )
         return NULL;
 
     /* Lock buffer in memory so it can't be paged out */