mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <>
To: Carlo Arenas <>
Cc: Junio C Hamano <>,
Subject: Re: [PATCH 2/3] approxidate: handle pending number for "specials"
Date: Tue, 6 Nov 2018 20:12:53 -0500	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Tue, Nov 06, 2018 at 04:48:28PM -0800, Carlo Arenas wrote:

> On Thu, Nov 1, 2018 at 10:24 PM Jeff King <> wrote:
> >
> >  static void date_yesterday(struct tm *tm, struct tm *now, int *num)
> >  {
> > +       *num = 0;
> the only caller (date_time) for this sends num = NULL, so this
> triggers a segfault.

Oof. Thanks for catching.

That's not the only caller. Usually this is called via a function
pointer from the "struct special" array. date_time() is just reusing the
other helper as a quick way to do a one-day subtraction.

> the only reference I could find to that apparently unused parameter comes from:
> 93cfa7c7a8 ("approxidate_careful() reports errorneous date string", 2010-01-26)
> and seems to indicate it is optional and therefore a check for NULL
> might make sense for all other cases

I think date_yesterday() is the only one of those special functions that
gets called like this. Here's what I think we should do to fix it (this
can go right on top of jk/misc-unused-fixes, which is already in next).

-- >8 --
Subject: [PATCH] approxidate: fix NULL dereference in date_time()

When we see a time like "noon", we pass "12" to our date_time() helper,
which sets the hour to 12pm. If the current time is before noon, then we
wrap around to yesterday using date_yesterday(). But unlike the normal
calls to date_yesterday() from approxidate_alpha(), we pass a NULL "num"
parameter. Since c27cc94fad (approxidate: handle pending number for
"specials", 2018-11-02), that causes a segfault.

One way to fix this is by checking for NULL. But arguably date_time() is
abusing our helper by passing NULL in the first place (and this is the
only case where one of these "special" parsers is used this way). So
instead, let's have it just do the 1-day subtraction itself. It's still
just a one-liner due to our update_tm() helper.

Note that the test added here is a little funny, as we say "10am noon",
which makes the "10am" seem pointless.  But this bug can only be
triggered when it the currently-parsed hour is before the special time.
The latest special time is "tea" at 1700, but t0006 uses a hard-coded
TEST_DATE_NOW of 1900. We could reset TEST_DATE_NOW, but that may lead
to confusion in other tests. Just saying "10am noon" makes this test

Reported-by: Carlo Arenas <>
Signed-off-by: Jeff King <>
 date.c          | 2 +-
 t/ | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/date.c b/date.c
index 7adce327a3..9bc15df6f9 100644
--- a/date.c
+++ b/date.c
@@ -929,7 +929,7 @@ static void date_yesterday(struct tm *tm, struct tm *now, int *num)
 static void date_time(struct tm *tm, struct tm *now, int hour)
 	if (tm->tm_hour < hour)
-		date_yesterday(tm, now, NULL);
+		update_tm(tm, now, 24*60*60);
 	tm->tm_hour = hour;
 	tm->tm_min = 0;
 	tm->tm_sec = 0;
diff --git a/t/ b/t/
index b7ea5fbc36..ffb2975e48 100755
--- a/t/
+++ b/t/
@@ -114,6 +114,7 @@ check_approxidate '15:00' '2009-08-30 15:00:00'
 check_approxidate 'noon today' '2009-08-30 12:00:00'
 check_approxidate 'noon yesterday' '2009-08-29 12:00:00'
 check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00'
+check_approxidate '10am noon' '2009-08-29 12:00:00'
 check_approxidate 'last tuesday' '2009-08-25 19:20:00'
 check_approxidate 'July 5th' '2009-07-05 19:20:00'

  reply	other threads:[~2018-11-07  1:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-02  5:22 [PATCH 0/3] mixed bag of -Wunused-parameter bugfixes Jeff King
2018-11-02  5:22 ` [PATCH 1/3] rev-list: handle flags for --indexed-objects Jeff King
2018-11-02  5:23 ` [PATCH 2/3] approxidate: handle pending number for "specials" Jeff King
2018-11-07  0:48   ` Carlo Arenas
2018-11-07  1:12     ` Jeff King [this message]
2018-11-07  2:08       ` Junio C Hamano
2018-11-02  5:23 ` [PATCH 3/3] pathspec: handle non-terminated strings with :(attr) Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).