patchFix date checking in case if time was not initialized.

Fix is_date() function failings in detection of correct date in case if time was not properly initialized.

From: Mike Gorchak <mike.gorchak.qnx@gmail.com>
Signed-off-by: Mike Gorchak <mike.gorchak.qnx@gmail.com>
---
 date.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Mike Gorchak <mike.gorchak.qnx@gmail.com> writes:

Please explain why this patch is needed and what problem this patch is trying to fix (if any) a bit better in the proposed log message. For example, on what input do we call this function with partially filled *r, and is that an error in the code, or is it an indication that the input has only been consumed partially?

tm_to_time_t() is designed to reject underspecified date input, and its callers call is_date() starting with partially filled tm on purpose to reject such input. Doesn't "fixing" partially filled tm before calling tm_to_time_t() inside is_date() break that logic?

function is_date() must not fail if time fields are not set. Currently is_date() invokes tm_to_time_t() which perform check of time fields. With these fixes t0006-date.sh test is no longer fail on these tests:

check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000' check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500' check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015' check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000' check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000' check_parse '2008-02-14 20:30:45 -05' '2008-02-14 20:30:45 -0500' check_parse '2008-02-14 20:30:45 -:30' '2008-02-14 20:30:45 +0000' check_parse '2008-02-14 20:30:45 -05:00' '2008-02-14 20:30:45 -0500'

By the way following test is always fails due to absence of EST5 timezone in timezones array in date.c module.

check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 -0500' EST5

According to logic is_date() have no any relations with time (hours/mins/secs). In the tests described above date is defined first, before time, so at the point when date is checked for correctness time is not defined yet.

Thanks.

Mike Gorchak <mike.gorchak.qnx@gmail.com> writes:

The thing that puzzles me is that nobody reported that the following fail on their platforms (and they do not fail for me on platforms I have to test in my real/virtual boxes).

So there must be something _else_ going on, and I cannot tell what that something else is from your code change nor from the log message. I'd like to see that explained.

Still puzzled...

Ok, check_parse calls function parse_date(), it calls parse_date_basic(), where following code is present:

	memset(&tm, 0, sizeof(tm));
	tm.tm_year = -1;
	tm.tm_mon = -1;
	tm.tm_mday = -1;
	tm.tm_isdst = -1;
	tm.tm_hour = -1;
	tm.tm_min = -1;
	tm.tm_sec = -1;

Almost all fields are set to -1. A bit later match_multi_number() is called. It parses the date and calls is_date() with partially filled tm structure, where all time fields: tm_hour, tm_min, tm_sec are set to -1. tm_to_time_t() function is called by is_date() and has special check:

	if (tm->tm_hour < 0 || tm->tm_min < 0 || tm->tm_sec < 0)
		return -1;

So is_date() always return negative result for the text string where date is placed before time like '2008-02-14 20:30:45'. It must fail on other platforms as well.

Mike Gorchak <mike.gorchak.qnx@gmail.com> writes:

Yes, it returns this -1 on other platforms, but...

... the input '2008-02-14 20:30:45' still parses fine for others (including me). That is what is puzzling.

A shot in the dark: perhaps your time_t is unsigned?

It also fails under Linux, but real problem is not here, it is just an unoptimal date parser.

Yeah, it was a headshot :) It really due to unsigned time_t. I will send two patches right now with fixes regarding unsigned time_t comparison.

Mike Gorchak <mike.gorchak.qnx@gmail.com> writes:

The test does _not_ fail. That if condition does return -1 on Linux and BSD, and making tm_to_time_t() return a failure, but the caller goes on, ending up with the right values in year/month/date in the tm struct, which is the primary thing the function is used for.

As you said, what is_date() wants to see is if the caller guessed the order of three combinations of year/month/date correctly, it cannot necessarily assume the caller already has seen the hour/minutes/seconds yet, so _temporarily_ feeding a valud set of values to hour/minutes/seconds when calling tm_to_time_t() is a good workaround. So the change in your patch sounds correct and use of a temporary tm to avoid contaminating the hour/minutes/seconds passed to the is_date() function while doing so looks good.

If your time_t is unsigned, the error returned from tm_to_time_t() will appear to be a time in a distant future, which will prevent is_date() to return "Yeah, you guessed the order of year, month, and date correctly" to its caller. The code would need to pick a safer mechanism to signal a failure from tm_to_time_t() to its callers.

I said it wrong, test itself is not failed, but numerous is_date() checks are failed due to incomplete time.