Re: [PATCH] ntp: make is_error_status() use its argument

[John Stultz]

On 05/12/2014 06:35 AM, George Spelvin wrote:
> It's an inline function always called with the global time_status as an
> argument, so there's zero functional difference, but the non-CONFIG_SMP
> version uses the passed-in argument, while the CONFIG_SMP one ignores
> its argument and uses the global.

CONFIG_NTP_PPS not CONFIG_SMP, right?

Good catch though, looks like the check code was re-factored out, but
someone forgot to use the local variable.

Adding Alexander since he submitted the pps logic.

If there's no objections, I'll queue this (with a more verbose commit
message) for 3.16

thanks
-john


>     
> Make it use the argument always; shorter variable names are good.
>
> Signed-off-by: George Spelvin <linux@xxxxxxxxxxx>
> ---
> While poking about in the code, I came across this rather odd bit.
> It looked worth fixing, on general principles.
>
> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
> index 419a52cecd..1a2aad3fff 100644
> --- a/kernel/time/ntp.c
> +++ b/kernel/time/ntp.c
> @@ -165,21 +165,21 @@ static inline void pps_set_freq(s64 freq)
>  
>  static inline int is_error_status(int status)
>  {
> -	return (time_status & (STA_UNSYNC|STA_CLOCKERR))
> +	return (status & (STA_UNSYNC|STA_CLOCKERR))
>  		/* PPS signal lost when either PPS time or
>  		 * PPS frequency synchronization requested
>  		 */
> -		|| ((time_status & (STA_PPSFREQ|STA_PPSTIME))
> -			&& !(time_status & STA_PPSSIGNAL))
> +		|| ((status & (STA_PPSFREQ|STA_PPSTIME))
> +			&& !(status & STA_PPSSIGNAL))
>  		/* PPS jitter exceeded when
>  		 * PPS time synchronization requested */
> -		|| ((time_status & (STA_PPSTIME|STA_PPSJITTER))
> +		|| ((status & (STA_PPSTIME|STA_PPSJITTER))
>  			== (STA_PPSTIME|STA_PPSJITTER))
>  		/* PPS wander exceeded or calibration error when
>  		 * PPS frequency synchronization requested
>  		 */
> -		|| ((time_status & STA_PPSFREQ)
> -			&& (time_status & (STA_PPSWANDER|STA_PPSERROR)));
> +		|| ((status & STA_PPSFREQ)
> +			&& (status & (STA_PPSWANDER|STA_PPSERROR)));
>  }
>  
>  static inline void pps_fill_timex(struct timex *txc)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/