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@xxxxxxxxx> writes:fix: prevent date underflow when using positive timezone offsetcc: Patrick Steinhardt ps@xxxxxx cc: Phillip Woodphillip.wood123@xxxxxxxxxYou'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@xxxxxxxxx/ 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.
