Re: [PATCH] compat: add compat functions for *at syscalls

[Linus Torvalds <torvalds@osdl.org>]

On Tue, 7 Feb 2006, Stephen Rothwell wrote:
> 
> *If* we do get is_compat_task(), what would be you reaction to something
> like this:
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index faf61c3..83d6cd1 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1877,6 +1877,8 @@ asmlinkage long sys_mkdirat(int dfd, con
>  	int error = 0;
>  	char * tmp;
>  
> +	if (is_compat_task())
> +		dfd = compat_sign_extend(dfd);

Oh f*ck, why do people do ugly code like this?

That's just about the nastiest thing I've ever seen.

If you want to go this way, do it with

	asmlinkage long sys_mkdirat(long __dfd, ..
	{
		int dfd = __dfd;

which is at least unconditional.

The thing is unconditionally doing the "skip upper bits" is _faster_ then 
the conditional test to see if you even need it.

Conditionalizing code like this is EVIL and STUPID.

However, what I suspect David was actually suggesting was to just have 
some trivial for just the compat functions. You can generate them 
automatically based on 
 - function name
 - signedness/unsignedness of each argument
with some preprocessor hackery.

So each architecture could have the following #defines:

	#define SARG(x) "sext " x "," x	 // Whatever the architecture asm is for sign-extending a register
	#define UARG(x)	"zext " x "," x
	#define JMP(x)	"jmp " x

	#define ARG1 "%r3"
	#define ARG2 "%r4"
	#define ARG3 "%r5"
	...

and then there would be a generic helper header:

	#define compat_fn1(n, x1) 		\
		asm("compat_" ## n ":\n\t"	\
			x1(ARG1) "\n\t"		\
			JMP(##n))

	#define compat_fn2(n, x1, x2) 		\
		asm("compat_" ## n ":\n\t"	\
			x1(ARG1) "\n\t"		\
			x2(ARG2) "\n\t"		\
			JMP(##n))

	#define compat_fn3(n, x1, x2) 		\
		asm("compat_" ## n ":\n\t"	\
			x1(ARG1) "\n\t"		\
			x2(ARG2) "\n\t"		\
			x3(ARG3) "\n\t"		\
			JMP(##n))

	... fn4..fn6 ..

	compat_fn4(sys_semctl, SARG, SARG, SARG, UARG);
	compat_fn6(sys_waitif, SARG, UARG, UARG, SARG, UARG);
	..

you get the idea - automatically generated stub assembly functions that 
zero-extend or sign-extend the arguments properly. Each architecture would 
need just a minimal set of "helper defines" to create the per-architecture 
assembly language.

(And no, I didn't test the above at all).

		Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/