Re: [RFC PATCH v2 01/11] time: Add do_settimeofday64() safe version(using timespec64)

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 30 Oct 2014, pang.xunlei wrote:

Your subject line

> time: Add do_settimeofday64() safe version(using timespec64)

is confusing at best.

time: Provide y2038 safe do_settimeofday() replacement

That explains what the patch is doing in a very precice way.

>  /**
> - * do_settimeofday - Sets the time of day
> - * @tv:		pointer to the timespec variable containing the new time
> + * do_settimeofday64 - Sets the time of day
> + * @ts:     pointer to the timespec64 variable containing the new time
>   *
>   * Sets the time of day to the new time and update NTP and notify hrtimers
> + * Safe version for 2038 safety.

I rather put this into the function description above:

+ * do_settimeofday64 - Sets the time of day. 2038 safe.

Way simpler to grep for.

>   */
> -int do_settimeofday(const struct timespec *tv)
> +int do_settimeofday64(const struct timespec64 *ts)
>  {
>  	struct timekeeper *tk = &tk_core.timekeeper;
> -	struct timespec64 ts_delta, xt, tmp;
> +	struct timespec64 ts_delta, xt;
>  	unsigned long flags;
>  
> -	if (!timespec_valid_strict(tv))
> +	if (!timespec64_valid_strict(ts))
>  		return -EINVAL;
>  
>  	raw_spin_lock_irqsave(&timekeeper_lock, flags);
> @@ -723,13 +724,12 @@ int do_settimeofday(const struct timespec *tv)
>  	timekeeping_forward_now(tk);
>  
>  	xt = tk_xtime(tk);
> -	ts_delta.tv_sec = tv->tv_sec - xt.tv_sec;
> -	ts_delta.tv_nsec = tv->tv_nsec - xt.tv_nsec;
> +	ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
> +	ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
>  
>  	tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts_delta));
>  
> -	tmp = timespec_to_timespec64(*tv);
> -	tk_set_xtime(tk, &tmp);
> +	tk_set_xtime(tk, ts);
>  
>  	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
>  
> @@ -741,6 +741,22 @@ int do_settimeofday(const struct timespec *tv)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL(do_settimeofday64);

EXPORT_SYMBOL_GPL please.

> +
> +/**
> + * do_settimeofday - Sets the time of day
> + * @tv:		pointer to the timespec variable containing the new time
> + *
> + * Sets the time of day to the new time and update NTP and notify hrtimers
> + * TODO: [2038 safety] should be replaced by do_settimeofday64().

The TODO is not to replace do_settimeofday() by do_settimeofday64(). The TODO
is to fix the call sites.

So instead of adding this you can simply say:

+ * do_settimeofday - Sets the time of day. Deprecated. Use do_settimeofday64()

We want things like this easy to find and not be burried in some
cryptic comment.

Thanks,

	tglx