Re: [PATCH v2] date: detect underflow when parsing dates with positive timezone offset

[darcy <acednes@gmail.com>]

> Without having a deep understanding of the code I don't quite see the
> connection between this change and the problem description. Is it
> necessary? If so, it might help to explain why it's needed in the commit
> message or in the code.

> ...I guess that
> checking for tm_hour is assuming that TZ offset should always come
> before the values necessary to compute the timestamp comes, but it
> smells like an unwarranted assumption and not explaining the change
> in the proposed log message is a bad sign.

This line has been reverted. The point was it would only parse the
timezone offset if it occurs after the time part of the date, but I
have realized that this is unrelated to the purpose of this change.

> You should also add at least one test.

Yep, thanks, added now to `t0006-date.sh`.

> Do we also want to check for overflows in the other direction (a large timestamp with a negative timezone offset)?

Is this something people want added? I am happy to implement this if
so, though it wasn't my original intention.

Issues with the commit message should also be resolved. Thank you
everyone for your patience :)

On 3/6/24 21:13, Junio C Hamano wrote:

> "darcy via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>>      fix: prevent date underflow when using positive timezone offset
>>      
>>      cc: Patrick Steinhardt ps@pks.im cc: Phillip Wood
>>      phillip.wood123@gmail.com
> You're expected to respond to review comments before you send in
> updated patches.  I didn't see the review comments responded to in
> the thread:
>
>    https://lore.kernel.org/git/pull.1726.git.git.1716801427015.gitgitgadget@gmail.com/
>
> Please see "A typical life cycle of a patch series" section of the
> SubmittingPatches document.
>
>    https://git.github.io/htmldocs/SubmittingPatches.html#patch-flow
>
> Step #3 and Step #4 are distinct.