Re: [PATCH 1/5] lguest host feedback tidyups

[Rusty Russell <rusty@rustcorp.com.au>]

On Mon, 2007-05-14 at 11:44 +1000, Rusty Russell wrote:
> On Sun, 2007-05-13 at 19:39 +0100, Al Viro wrote:
> > >  static void push_guest_stack(struct lguest *lg, u32 __user **gstack, u32 val)
> > >  {
> > > -	lgwrite_u32(lg, (u32)--(*gstack), val);
> > > +	lgwrite_u32(lg, (__force u32)--(*gstack), val);
> > >  }
> > 
> > Now, _that_ is just plain dumb.  Why not declare that lgwrite_u32() as taking
> > u32 __user * as argument and kill the casts?
> 
> Last I tried, this turns out to create even more casts.

But after thinking harder... changing the gstack type gets rid of three
casts, including that one.  Thanks!

How's this?
Rusty.
== 
Reduce number of casts in set_guest_interrupt().

Al Viro pointed out the ugly cast in push_lguest_stack(); as this is
only called from set_guest_interrupt() which casts the arg in the
first place, let's stick with unsigned long the whole way through.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff -r 16e417365c64 drivers/lguest/interrupts_and_traps.c
--- a/drivers/lguest/interrupts_and_traps.c	Mon May 14 15:05:48 2007 +1000
+++ b/drivers/lguest/interrupts_and_traps.c	Mon May 14 15:22:38 2007 +1000
@@ -16,24 +16,25 @@ static int idt_present(u32 lo, u32 hi)
 	return (hi & 0x8000);
 }
 
-static void push_guest_stack(struct lguest *lg, u32 __user **gstack, u32 val)
-{
-	lgwrite_u32(lg, (unsigned long)--(*gstack), val);
+static void push_guest_stack(struct lguest *lg, unsigned long *gstack, u32 val)
+{
+	*gstack -= 4;
+	lgwrite_u32(lg, *gstack, val);
 }
 
 static void set_guest_interrupt(struct lguest *lg, u32 lo, u32 hi, int has_err)
 {
-	u32 __user *gstack;
+	unsigned long gstack;
 	u32 eflags, ss, irq_enable;
 
 	/* If they want a ring change, we use new stack and push old ss/esp */
 	if ((lg->regs->ss&0x3) != GUEST_PL) {
-		gstack = (u32 __user *)guest_pa(lg, lg->esp1);
+		gstack = guest_pa(lg, lg->esp1);
 		ss = lg->ss1;
 		push_guest_stack(lg, &gstack, lg->regs->ss);
 		push_guest_stack(lg, &gstack, lg->regs->esp);
 	} else {
-		gstack = (u32 __user *)guest_pa(lg, lg->regs->esp);
+		gstack = guest_pa(lg, lg->regs->esp);
 		ss = lg->regs->ss;
 	}
 
@@ -53,7 +54,7 @@ static void set_guest_interrupt(struct l
 
 	/* Change the real stack so switcher returns to trap handler */
 	lg->regs->ss = ss;
-	lg->regs->esp = (unsigned long)gstack + lg->page_offset;
+	lg->regs->esp = gstack + lg->page_offset;
 	lg->regs->cs = (__KERNEL_CS|GUEST_PL);
 	lg->regs->eip = idt_address(lo, hi);