<div dir="ltr"><div class="gmail_extra"><br></div><div class="gmail_quote">On 20 April 2015 at 16:42, Richard Cochran <span dir="ltr"><<a href="mailto:richardcochran@gmail.com" target="_blank">richardcochran@gmail.com</a>></span> wrote:<br><blockquote style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid" class="gmail_quote"><span>On Mon, Apr 20, 2015 at 01:57:39PM +0800, Baolin Wang wrote:<br>
<br>
> @@ -911,18 +907,14 @@ retry:<br>
>               return -EINVAL;<br>
><br>
>       kc = clockid_to_kclock(timr->it_clock);<br>
> -     if (WARN_ON_ONCE(!kc || (!kc->timer_set && !kc->timer_set64))) {<br>
> +     if (WARN_ON_ONCE(!kc || !kc->timer_set64)) {<br>
>               error = -EINVAL;<br>
>       } else {<br>
> -             if (kc->timer_set64) {<br>
> -                     new_spec64 = itimerspec_to_itimerspec64(new_spec);<br>
> -                     error = kc->timer_set64(timr, flags, &new_spec64,<br>
> -                                             &old_spec64);<br>
> -                     if (old_setting)<br>
> -                             old_spec = itimerspec64_to_itimerspec(old_spec64);<br>
> -             } else {<br>
> -                     error = kc->timer_set(timr, flags, &new_spec, rtn);<br>
> -             }<br>
> +             new_spec64 = itimerspec_to_itimerspec64(new_spec);<br>
> +             error = kc->timer_set64(timr, flags, &new_spec64,<br>
> +                                     &old_spec64);<br>
<br>
</span>This statement can fit on one line.<br>
<span><br>
> +             if (old_setting)<br>
> +                     old_spec = itimerspec64_to_itimerspec(old_spec64);<br>
>       }<br>
><br>
>       unlock_timer(timr, flag);<br>
<br>
</span><span>> @@ -1057,14 +1045,13 @@ SYSCALL_DEFINE2(clock_gettime, const clockid_t, which_clock,<br>
>       if (!kc)<br>
>               return -EINVAL;<br>
><br>
> -     if (kc->clock_get64) {<br>
> -             error = kc->clock_get64(which_clock, &kernel_tp64);<br>
> -             kernel_tp = timespec64_to_timespec(kernel_tp64);<br>
> -     } else {<br>
> -             error = kc->clock_get(which_clock, &kernel_tp);<br>
> -     }<br>
> +     error = kc->clock_get64(which_clock, &kernel_tp64);<br>
> +     if (!error)<br>
> +             return error;<br>
<br>
</span>Wrong test, should be: if (error) ...<br>
<span><br>
> +<br>
> +     kernel_tp = timespec64_to_timespec(kernel_tp64);<br>
><br>
> -     if (!error && copy_to_user(tp, &kernel_tp, sizeof (kernel_tp)))<br>
<br>
</span>The (!error && ...) was correct here!<br>
<span><br>
> +     if (copy_to_user(tp, &kernel_tp, sizeof (kernel_tp)))<br>
>               error = -EFAULT;<br>
><br>
>       return error;<br>
<br>
</span>You can simplify this like so:<br>
<br>
        return copy_to_user(tp, &kernel_tp, sizeof(kernel_tp)) ? -EFAULT : 0;<br>
<span><br>
> @@ -1104,14 +1091,13 @@ SYSCALL_DEFINE2(clock_getres, const clockid_t, which_clock,<br>
>       if (!kc)<br>
>               return -EINVAL;<br>
><br>
> -     if (kc->clock_getres64) {<br>
> -             error = kc->clock_getres64(which_clock, &rtn_tp64);<br>
> -             rtn_tp = timespec64_to_timespec(rtn_tp64);<br>
> -     } else {<br>
> -             error = kc->clock_getres(which_clock, &rtn_tp);<br>
> -     }<br>
> +     error = kc->clock_getres64(which_clock, &rtn_tp64);<br>
> +     if (!error)<br>
> +             return error;<br>
<br>
</span>Also wrong.<br>
<span><br>
> +<br>
> +     rtn_tp = timespec64_to_timespec(rtn_tp64);<br>
><br>
> -     if (!error && tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp)))<br>
> +     if (tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp)))<br>
>               error = -EFAULT;<br>
><br>
>       return error;<br>
> --<br>
> 1.7.9.5<br>
><br>
<br>
</span>Thanks,<br>
Richard<br>
</blockquote></div><p> </p><div>Thanks for your comments, i'll fix these mistakes in next patch series.</div><div class="gmail_extra"><br><br clear="all"><br>-- <br></div><div class="gmail_signature"><div dir="ltr"><div>Baolin.wang<br></div>Best Regards<br></div></div><div class="gmail_extra">
</div></div>