Re: [PATCH 11/11] k_clock:Remove the 32bit methods with timespec type
From: Richard Cochran <richardcochran@gmail.com>
Date: 2015-04-20 08:42:43
Also in:
linux-arch, linux-s390, lkml, netdev
On Mon, Apr 20, 2015 at 01:57:39PM +0800, Baolin Wang wrote:
quoted hunk ↗ jump to hunk
@@ -911,18 +907,14 @@ retry: return -EINVAL; kc = clockid_to_kclock(timr->it_clock); - if (WARN_ON_ONCE(!kc || (!kc->timer_set && !kc->timer_set64))) { + if (WARN_ON_ONCE(!kc || !kc->timer_set64)) { error = -EINVAL; } else { - if (kc->timer_set64) { - new_spec64 = itimerspec_to_itimerspec64(new_spec); - error = kc->timer_set64(timr, flags, &new_spec64, - &old_spec64); - if (old_setting) - old_spec = itimerspec64_to_itimerspec(old_spec64); - } else { - error = kc->timer_set(timr, flags, &new_spec, rtn); - } + new_spec64 = itimerspec_to_itimerspec64(new_spec); + error = kc->timer_set64(timr, flags, &new_spec64, + &old_spec64);
This statement can fit on one line.
+ if (old_setting) + old_spec = itimerspec64_to_itimerspec(old_spec64); } unlock_timer(timr, flag);
quoted hunk ↗ jump to hunk
@@ -1057,14 +1045,13 @@ SYSCALL_DEFINE2(clock_gettime, const clockid_t, which_clock, if (!kc) return -EINVAL; - if (kc->clock_get64) { - error = kc->clock_get64(which_clock, &kernel_tp64); - kernel_tp = timespec64_to_timespec(kernel_tp64); - } else { - error = kc->clock_get(which_clock, &kernel_tp); - } + error = kc->clock_get64(which_clock, &kernel_tp64); + if (!error) + return error;
Wrong test, should be: if (error) ...
+ + kernel_tp = timespec64_to_timespec(kernel_tp64); - if (!error && copy_to_user(tp, &kernel_tp, sizeof (kernel_tp)))
The (!error && ...) was correct here!
+ if (copy_to_user(tp, &kernel_tp, sizeof (kernel_tp))) error = -EFAULT; return error;
You can simplify this like so: return copy_to_user(tp, &kernel_tp, sizeof(kernel_tp)) ? -EFAULT : 0;
quoted hunk ↗ jump to hunk
@@ -1104,14 +1091,13 @@ SYSCALL_DEFINE2(clock_getres, const clockid_t, which_clock, if (!kc) return -EINVAL; - if (kc->clock_getres64) { - error = kc->clock_getres64(which_clock, &rtn_tp64); - rtn_tp = timespec64_to_timespec(rtn_tp64); - } else { - error = kc->clock_getres(which_clock, &rtn_tp); - } + error = kc->clock_getres64(which_clock, &rtn_tp64); + if (!error) + return error;
Also wrong.
+ + rtn_tp = timespec64_to_timespec(rtn_tp64); - if (!error && tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp))) + if (tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp))) error = -EFAULT; return error; -- 1.7.9.5
Thanks, Richard