On Monday 16 January 2006 06:59, Shaun Pereira wrote:

I was wondering if this the compat_sock_get_timestamp function is
needed? If I were to remove the SIOCGSTAMP case from the
compat_x25_ioctl function, then a SIOCGSTAMP ioctl system call would
return -ENOIOCTLCMD which could  then be handled by do_siocgstamp
handler in the ioctl32_hash_table? (fs/compat_ioctl.c)
In which case I could remove this patch from the rest of the series.

Yes, that would also work, as I already mentioned (or tried to)
in one of my earlier comments. I would prefer to have this patch
though, because in the long term, I think we should migrate more
stuff away from the hash table and having the function there
means that others can use it as well.

+       err = -EFAULT;
+       if(access_ok(VERIFTY_WRITE, ctv, sizeof(*ctv))) {
+               err = __put_user(sk->sk_stamp.tv_sec, &ctv->tv_sec);
+               err != __put_user(sk->sk_stamp.tv_usec, &ctv->tv_usec);
+       }
+       return err;
+}

This copies the correct data down to user space now, but might result
in returning an invalid error code.
In the second line you now have 'err != __put_user(...);', which is
a comparison, not an assignment!
For readability, I would simply write that as:

	ret = 0;
	if (put_user(sk->sk_stamp.tv_sec, &ctv->tv_sec) |
	    put_user(sk->sk_stamp.tv_usec, &ctv->tv_usec))
		err = -EFAULT;

You can also write it like your code, but with '|' instead of '!', but
that requires the additional knowledge that __put_user can only ever
return '0' or '-EFAULT' itself and that the bitwise or of those is
therefore also one of these two.

	Arnd <><