linux-kernel - Re: [PATCH] ntp: make is_error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 12 May 2014 10:54:29 -0700
From:	John Stultz <john.stultz@...aro.org>
To:	George Spelvin <linux@...izon.com>
CC:	linux-kernel@...r.kernel.org,
	Alexander Gordeev <lasaine@....cs.msu.su>
Subject: Re: [PATCH] ntp: make is_error_status() use its argument

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@...izon.com>
> ---
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/