gmtime() in library is busted by logic error

[GoogleCodeExporter]

What steps will reproduce the problem?

1. Compile the attached test.c program using the following script:

propeller-elf-gcc -O2 -mfcache -save-temps -mlmm -o test.o -c test.c
propeller-elf-gcc -Xlinker -Map=DS1302_test.map -mlmm -fno-exceptions -fno-rtti 
-o DS1302_test.elf test.o  -s

2. Run the program by loading it to a propeller.

What is the expected output? What do you see instead?

I expect to see the program spew out a bunch of date conversions without the 
programming printing out any errors.  Instead all dates have conversion errors!

Example:

Date to test:    01/01/1970 00:00:00
Converted time_t:           0   (now set the clock)
Sytem time:                 0
Convert back:    01/00/1970 00:00:00
Error: gmtime mismatch!
Day of year:     0
Day of week:     4
Using strftime:  01/00/1970 00:00:00
Error: strftime mismatch!

What version of the product are you using? On what operating system?

propgcc-0.2.2

Please provide any additional information below.

The problem is in the _gmtime_r routine.  The code says it is from the MiNT 
library, but looking at the MiNT source at 
http://cd.textfiles.com/geminiatari/FILES/MINT/MNTLIB18/LOCALTIM.C, it is 
obvious that the _gmtime routine was changed from the MiNT source.

The first problem is the missing  " + 1" in the "stm->tm_mday = mday + 1;" line.

A second and more troubling problem is that the new "align on post-1972 
boundary" code is just wrong.  You can't align starting at 1972, because the 
leap day hasn't happened yet.  You can only align starting at the beginning of 
1973, and knock off quad-years, but afterwards (with the "up to 4 years" 
remainder) you still have to knock off a year at a time to account for a 
possible year with a leap day.

Attached please find code that works if you compile with the following commands:

propeller-elf-gcc -O2 -mfcache -save-temps -mlmm -o test.o -c test.c
propeller-elf-gcc -O2 -mfcache -save-temps -mlmm -o localtim.o -c localtim.c
propeller-elf-gcc -Xlinker -Map=DS1302_test.map -mlmm -fno-exceptions -fno-rtti 
-o DS1302_test.elf test.o localtim.o  -s

Note that this code loses in the "space vs time" tradeoff - it's quicker to 
run, but takes up more room.  In my opinion, I'd prefer to have the new "align 
on post-1972 boundary" code removed leaving just the original MiNT code.  Sure 
it's a little slower, but personally I think gmtime is just not run that often 
and I'd rather have it be slower but smaller.

On last thing to point out: I've attached the diffs of this code to the code 
checked in at 
http://code.google.com/p/propgcc/source/browse/lib/time/localtim.c.  If you 
notice in the diff, there's an addition of a "struct tm *gmtime()" routine at 
"@@ -179,6 +190,11 @@".

The interesting thing is that I did not add this code.  This extra line appears 
in the library found at 
http://code.google.com/p/propgcc/downloads/detail?name=propgcc-libsrc_2011-12-21
.zip, but this particular change is not checked in to the repository at 
http://code.google.com/p/propgcc/source/browse/lib/time/localtim.c.

It's a little disconcerting to see these out of sync.

Original issue reported on code.google.com by tjstefan...@charter.net on 2 Feb 2012 at 7:25

Attachments:

• test.c
• localtim.c
• localtim.diffs

[GoogleCodeExporter]

Thank you for this detailed bug report (and test case!). I'm going to take a 
look at another look at the time functions; obviously we need to fix these.

I'm surprised that the addition of gmtime() didn't show up for you in the 
repository. I see it there now (it was added by a check in on Dec. 11). Perhaps 
your local repository is out of date or on a branch?

We do appreciate your help with testing and fixing these functions. Thanks for 
the bug reports!

Eric

Original comment by ersm...@hfx.eastlink.ca on 4 Feb 2012 at 9:40

• Changed state: Accepted

[GoogleCodeExporter]

Eric,

Sorry for the noise on the missing gmtime declaration. I created the code using 
a fresh source file I got from this Google Code website, but obviously I pulled 
the wrong branch.  You are correct that the repository had the correct code.

Thanks for the fix!

- Ted

Original comment by tjstefan...@charter.net on 5 Feb 2012 at 4:01

[GoogleCodeExporter]

I'm glad that we got the code sorted out :-). As you've probably seen, I've 
checked in an updated gmtime() that works correctly, and some tests in 
lib/Test/time.c to make sure we don't regress on this.

Thanks for your help,
Eric

Original comment by ersm...@hfx.eastlink.ca on 7 Feb 2012 at 1:30

• Changed state: Fixed