[tz] [PROPOSED] Simplify zdump average-taking

[John Hawkinson]

I don't mean to single out this diff, but: why are we making changes like this?
I cannot imagine that the width of a single instruction in zdump (of all things!) matters to...literally anyone?
[ If someone has an embedded system that spends its time flat-out running zdump to control an elevator or something, I want to hear about it! ]
And the risk of these fixes introducing subtle errors or not being carefully reviewed seems far higher than the benefit?

I think I am missing something but could someone explain the perspective of why these kinds of changes should be made?

Thanks.

--
jhawk at alum.mit.edu
John Hawkinson


Paul Eggert via tz <tz at iana.org> wrote on Wed, 30 Nov 2022
at 20:32:26 EST in <20221201013226.90512-1-eggert at cs.ucla.edu>:


> * zdump.c (hunt): Simplify slightly, so that one more instruction
> can be int width rather than time_t width if that’s what the
> compiler prefers.  No need to mention C89 in the comment, as C89
> support is going away soon.  (The code still happens to work in
> C89, for what it’s worth.)
> ---
>  zdump.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/zdump.c b/zdump.c
> index a020102..04953dc 100644
> --- a/zdump.c
> +++ b/zdump.c
> @@ -732,13 +732,9 @@ hunt(timezone_t tz, time_t lot, time_t hit, bool only_ok)
>  
>  	for ( ; ; ) {
>  		/* T = average of LOT and HIT, rounding down.
> -		   Avoid overflow, even on oddball C89 platforms
> -		   where / rounds down and TIME_T_MIN == -TIME_T_MAX
> -		   so lot / 2 + hit / 2 might overflow.  */
> -		time_t t = (lot / 2
> -			    - ((lot % 2 + hit % 2) < 0)
> -			    + ((lot % 2 + hit % 2) == 2)
> -			    + hit / 2);
> +		   Avoid overflow.  */
> +		int rem_sum = lot % 2 + hit % 2;
> +		time_t t = (rem_sum == 2) - (rem_sum < 0) + lot / 2 + hit / 2;
>  		if (t == lot)
>  			break;
>  		tm_ok = my_localtime_rz(tz, &t, &tm) != NULL;
> -- 
> 2.38.1
>