Re: rename 'unused' in sys_iopl to somethign else, since it is used

Re: rename 'unused' in sys_iopl to somethign else, since it is used

[Rusty Russell]

In message <20020728141256.GA9573@alhambra.actcom.co.il> you write:
> sys_iopl on i386 accepts one parameter, 'unsigned long unused'. Then
> it goes ahead and uses it to get a pointer to struct pt_regs. This
> patch changes its name to 'location'.=20

Hmmm.... Andi, the x86_64 version is different: do you really push one
arg on the stack then the regs?  Please check...

> diff -Nru a/arch/i386/kernel/ioport.c b/arch/i386/kernel/ioport.c
> --- a/arch/i386/kernel/ioport.c	Sun Jul 28 17:08:54 2002
> +++ b/arch/i386/kernel/ioport.c	Sun Jul 28 17:08:54 2002
> @@ -102,9 +102,9 @@
>   * code.
>   */
>  
> -asmlinkage int sys_iopl(unsigned long unused)
> +asmlinkage int sys_iopl(unsigned long location)
>  {
> -	struct pt_regs * regs =3D (struct pt_regs *) &unused;
> +	struct pt_regs * regs =3D (struct pt_regs *) &location;=20
>  	unsigned int level =3D regs->ebx;
>  	unsigned int old =3D (regs->eflags >> 12) & 3;
>  

It also then rereads the arg out of regs->ebx.  I prefer the following
patch, which makes it obvious that the arg *is* required, and what it
means:

diff -urpN -I \$.*\$ --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal linux-2.5.29/arch/i386/kernel/ioport.c working-2.5.29-iopl/arch/i386/kernel/ioport.c
--- linux-2.5.29/arch/i386/kernel/ioport.c	Mon Jun 17 23:19:16 2002
+++ working-2.5.29-iopl/arch/i386/kernel/ioport.c	Tue Jul 30 10:58:38 2002
@@ -102,10 +102,9 @@ asmlinkage int sys_ioperm(unsigned long 
  * code.
  */
 
-asmlinkage int sys_iopl(unsigned long unused)
+asmlinkage int sys_iopl(unsigned int level)
 {
-	struct pt_regs * regs = (struct pt_regs *) &unused;
-	unsigned int level = regs->ebx;
+	struct pt_regs * regs = (struct pt_regs *) &level;
 	unsigned int old = (regs->eflags >> 12) & 3;
 
 	if (level > 3)

Thanks!
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: rename 'unused' in sys_iopl to somethign else, since it is used

[Andi Kleen]

On Tue, Jul 30, 2002 at 11:02:33AM +1000, Rusty Russell wrote:
> In message <20020728141256.GA9573@alhambra.actcom.co.il> you write:
> > sys_iopl on i386 accepts one parameter, 'unsigned long unused'. Then
> > it goes ahead and uses it to get a pointer to struct pt_regs. This
> > patch changes its name to 'location'.=20
> 
> Hmmm.... Andi, the x86_64 version is different: do you really push one
> arg on the stack then the regs?  Please check...

level is in a register (rdi), but pt_regs is on the stack. The calling 
convention automatically switches to stack location for structures bigger 
than one register.  

On i386 it is probably cleanest when you need both pt_regs and arguments
to declare pt_regs only and get the first argument directly out of regs->ebx.


-Andi