Re: [PATCH 1/3] date: add helpers to convert between "+HHMM" timezones and minutes

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 30, 2026, 19:54 UTC
Message-ID: <xmqqa4oya51p.fsf@gitster.g>
In-Reply-To: <CAOLa=ZQRDVL2Djh4du1zWGg_ABZTyzaWDYYb0PDg3EXAfpn7bA@mail.gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 9 quoted lines

>> -	int minutes;
>> +	int minutes = tz < 0 ? -tz : tz;
>
> This is the part which we could skip as we're C99 compliant, but keeping
> to be on the safe side.
>
>> +	minutes = (minutes / 100) * 60 + (minutes % 100);
>> +	return tz < 0 ? -minutes : minutes;
>> +}

I was wondering exactly the same thing yesterday.

As written, it is clear even to those unfamiliar with the C89/C99 signed division rules, because we deal only with non-negative numbers, which is a plus. The fewer things readers need to worry about, the better.

Show 7 quoted lines

>>  	offset /= 60; /* in minutes */
>> -	offset = (offset % 60) + ((offset / 60) * 100);
>> -	return offset * eastwest;
>> +	return minutes_to_tz(offset * eastwest);
>
> While mathematically it's the same, but shouldn't this have been
> `minutes_to_tz(offset) * eastwest`?

The way you suggest is more faithful rewrite of the original.

Thanks.