On 20 April 2015 at 16:42, Richard Cochran <richardcoch...@gmail.com> wrote:
> On Mon, Apr 20, 2015 at 01:57:39PM +0800, Baolin Wang wrote:
>
> > @@ -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);
>
> > @@ -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;
>
> > @@ -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
>
Thanks for your comments, i'll fix these mistakes in next patch series.
--
Baolin.wang
Best Regards
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
