On Thursday 09 July 2015 17:02:47 Bamvor Zhang Jian wrote:
On 07/09/2015 04:09 AM, John Stultz wrote:
On Mon, Jun 29, 2015 at 7:23 AM, Bamvor Zhang Jian bamvor.zhangjian@linaro.org wrote:
+int get_timeval64(struct timeval64 *tv,
const struct __kernel_timeval __user *utv)+{
struct __kernel_timeval ktv;int ret;ret = copy_from_user(&ktv, utv, sizeof(ktv));if (ret)return -EFAULT;tv->tv_sec = ktv.tv_sec;if (!IS_ENABLED(CONFIG_64BIT)+#ifdef CONFIG_COMPAT
|| is_compat_task()+#endif
These sorts of ifdefs are to be avoided inside of functions.
Instead, it seems is_compat_task() should be defined to 0 in the !CONFIG_COMPAT case, so you can avoid the ifdefs and the compiler can still optimize it out.
I add this ifdef because I got compile failure on arm platform. This file do not include the <linux/compat.h> directly. And in arm64, compat.h is included implicitily. So, I am not sure what I should do here. Include <linux/compat.h> in this file directly or add a this check at the beginning of this file?
#ifndef is_compat_task #define is_compat_task() (0) #endif
Actually I think we can completely skip this test here: Unlike timespec, timeval is defined in a way that always lets user space use a 64-bit type for the microsecond portion (suseconds_t tv_usec).
I think we should simplify this case and just assume that user space does exactly that, and treat a tv_usec value with a nonzero upper half as an error.
I would also keep this function local to the ppdev driver, in order to not proliferate this to generic kernel code, but that is something we can debate, based on what other drivers need. For core kernel code, we should not need a get_timeval64 function because all system calls that pass a timeval structure are obsolete and we don't need to provide 64-bit time_t variants of them.
Arnd