Patchwork linux-user/syscall.c - don't add GUEST_BASE to NULL pointer

login
register
mail settings
Submitter Jan-Simon Möller
Date Aug. 25, 2009, 11:37 p.m.
Message ID <200908260137.48781.dl9pf@gmx.de>
Download mbox | patch
Permalink /patch/32100/
State Superseded
Headers show

Comments

Jan-Simon Möller - Aug. 25, 2009, 11:37 p.m.
Thinking a bit more about this, I wonder if g2h(x) shouldn't itself always 
return NULL on x = NULL ? 

Something like:

Signed-off-by:  Jan-Simon Möller  <dl9pf@gmx.de>



I read the comment above, but before replacing it in user-mode (if possible), 
we should fix it ;) .


Best,
Jan-Simon
Riku Voipio - Aug. 26, 2009, 1:40 p.m.
On Wed, Aug 26, 2009 at 01:37:48AM +0200, Jan-Simon Möller wrote:
> Thinking a bit more about this, I wonder if g2h(x) shouldn't itself always 
> return NULL on x = NULL ? 

I agree this seems like a a better idea than modifying the users of g2h.

> Something like:
> 
> Signed-off-by:  Jan-Simon Möller  <dl9pf@gmx.de>
> 
> diff --git a/cpu-all.h b/cpu-all.h
> index 1a6a812..631f678 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -633,7 +633,7 @@ extern int have_guest_base;
>  #endif
> 
>  /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
> -#define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
> +#define g2h(x) ( !x ? NULL:((void *)((unsigned long)(x) + GUEST_BASE)))
>  #define h2g(x) ({ \
>      unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
>      /* Check if given address fits target address space */ \
> 
> 
> I read the comment above, but before replacing it in user-mode (if possible), 
> we should fix it ;) .
> 
> 
> Best,
> Jan-Simon
> 
> 
>
Jan-Simon Möller - Aug. 26, 2009, 7:58 p.m.
Am Mittwoch 26 August 2009 15:40:43 schrieb Riku Voipio:
> On Wed, Aug 26, 2009 at 01:37:48AM +0200, Jan-Simon Möller wrote:
> > Thinking a bit more about this, I wonder if g2h(x) shouldn't itself
> > always return NULL on x = NULL ?
>
> I agree this seems like a a better idea than modifying the users of g2h.
>
Ok, then please apply to master.

Best,
Jan-Simon
Jan-Simon Möller - Aug. 28, 2009, 1:20 p.m.
Am Mittwoch 26 August 2009 15:40:43 schrieb Riku Voipio:
> On Wed, Aug 26, 2009 at 01:37:48AM +0200, Jan-Simon Möller wrote:
> > Thinking a bit more about this, I wonder if g2h(x) shouldn't itself
> > always return NULL on x = NULL ?
>
> I agree this seems like a a better idea than modifying the users of g2h.
>
> > Something like:
> >
> > Signed-off-by:  Jan-Simon Möller  <dl9pf@gmx.de>
> >
> > diff --git a/cpu-all.h b/cpu-all.h
> > index 1a6a812..631f678 100644
> > --- a/cpu-all.h
> > +++ b/cpu-all.h
> > @@ -633,7 +633,7 @@ extern int have_guest_base;
> >  #endif
> >
> >  /* All direct uses of g2h and h2g need to go away for usermode softmmu. 
> > */ -#define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
> > +#define g2h(x) ( !x ? NULL:((void *)((unsigned long)(x) + GUEST_BASE)))
> >  #define h2g(x) ({ \
> >      unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
> >      /* Check if given address fits target address space */ \
> >
> >

Take the first patch for syscall.c / mount .

Unfortunately, the above one has side-effects where functions rely on a 
shifted NULL pointer ... 

Best,
Jan-Simon

Patch

diff --git a/cpu-all.h b/cpu-all.h
index 1a6a812..631f678 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -633,7 +633,7 @@  extern int have_guest_base;
 #endif

 /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
-#define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
+#define g2h(x) ( !x ? NULL:((void *)((unsigned long)(x) + GUEST_BASE)))
 #define h2g(x) ({ \
     unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
     /* Check if given address fits target address space */ \