[Gnucash-changes] r12308 - Avoid testing dates near or after 2038

Chris Shoemaker c.shoemaker at cox.net
Mon Jan 9 18:37:22 EST 2006


On Mon, Jan 09, 2006 at 10:23:43PM +0000, Neil Williams wrote:
> On Monday 09 January 2006 9:06 pm, Chris Shoemaker wrote:
> > Author: chris
> > Date: 2006-01-09 16:06:45 -0500 (Mon, 09 Jan 2006)
> > New Revision: 12308
> > Trac: http://svn.gnucash.org/trac/changeset/12308
> >
> > Modified:
> >    gnucash/trunk/src/engine/test/test-date.c
> > Log:
> > Avoid testing dates near or after 2038, because the qof parsing functions
> > fail.
> 
> Are you sure you've identified the problem correctly?

Yes.  gnc_iso8601_to_timespec_gmt relies on mktime.  You're right
about it being a mess.  There are several things I would do
differently.

First, I hope you realize that _most_ of what that function does is
supplied by strptime.  The only thing that makes your function
different is that it handles the fractional seconds.  (And perhaps
it's _supposed to handle dates after 2038, but it doesn't explicitly
claim to, and it doesn't.)  This function could be simpified by using
strptime to handle everything except the fractional seconds.

OTOH, if you _really_ want to handle >2038, then you actually need to
do the conversion to seconds yourself, which really does require
summing the days in every year from 1970 to $date, and catching every
leap year.

My best advice is: don't do it.  Don't suck date parsing functions
into libqof.  They're yucky and not related to the core idea of qof.
And, your function doesn't even begin to handle the full range of iso
8601, so it a bit misnamed.  (E.g. Both '.' and ',' are supposed to be
supported as decimal/fraction separator, with ',' preferred.)

It would make a lot more sense if glib offered a 64-bit version of
mktime.  And, from gnucash's perspective, if both glib and qof offered
date-parsing functions, I'd much rather use glib's.

IMO, Gnucash can live with these functions as-is for now, and look for
a better solution later.

> QOF can handle dates far into the past and future, it may just be that the 
> test-date routine is using the wrong method to test.

Timespec is not the limitation.  If you really want to support those
dates, you have to avoid using mktime in the parsing functions.

> If you've got ideas for a better test of the date handling routines OR if you 
> find that the bug is actually in gnc_iso8601_to_timespec_gmt - let me know 
> (it's a beast of a function and has a number of nasty surprises hidden inside 
> it - it is certainly where I'd start hunting this kind of bug, if it exists).

Yep, that's where it is.  I got about 20 min. into debugging this
function, and decided it basically needs a re-write.  Then I decided I
don't care that much.  I'd rather disable the test for now, and wait
for a sane parsing function to show up somewhere else, then re-enable
the tests.

> 
> At the moment, your change simply avoids your reported issue.
> 
> My problems with that approach are:
> 1. I don't see this issue on my test systems so I'm not sure it has been 
> properly identified.

Perhaps you're on a 64 bit system?  or your ranlib init is different
than mine.  It's easy enough to just add an explicit test for a date
after 2038.

> 2. Your change simply avoids testing these dates - even when the tests would 
> appear fine on my systems.
> 3. If there is a bug in gnc_iso8601_to_timespec_gmt or similar, I want to fix 
> it, not ignore it!

That's ambitious, but is it _really_ related to qof?  If you really
want this, I'd recommend fixing it up right and submitting it for
inclusion into glib.

> 
> I will not be porting this change to the test routine to the comparable test 
> routine in external QOF. That continues to operate just fine.
> 
> If you can generate erroneous date handling in external QOF, please file a bug 
> report for QOF (not gnucash) at SourceForge and I'll work on a fix for the 
> next QOF release.

I don't have any plans to test with external qof at the moment, but
I'd recommend simply adding:

check_conversion ("2040-01-01 01:00:00 +0000", ts);

to your test routine.

-chris


More information about the gnucash-devel mailing list