GNOME Bugzilla – Bug 628137
gdatetime.c
Last modified: 2010-08-27 16:39:47 UTC
gdatetime should use a GDate for storing the date part of the timestamp. There are simply too many details to get wrong to have two copies of such code in glib. And gdatetime gets it wrong. Just take a look at g_date_time_add_dmy to get an idea of how wrong. In no particular order: 1. There is no effort of handling overflow. If someone adds too many years, the result is garbage and there does not seem to be a way to return an error anyway. 2. If you add 0 days to 2000-02-02, it would appear that you move one day back. If you add 0 days to 2000-02-01, the code will work with the zeroth day of that month -- I don't want to guess what that will do. 3. It uses C99 reserved identifiers (Section 7.1.3) like __year for no good reason. 4. If you add MIN_INT months, you will overflow the negation in ABS. In practice that mean you get nowhere. 5. If you add MAX_INT months, the "for" loop will take a long time. 6. No-one has thought about end-of-month effects like "add a month and a day to 2000-01-31". What you get is a total accident of the implementation. Undocumented, of course. 7. Via g_date_time_add_days_internal, ->period is accessed. That is a three-bit bitfield with implementation-dependent signedness. 8. The docs could use a bit of proof-reading. 9. It gets September 1752 wrong. And probably anything before that. If you look at another function, like g_date_time_get_hour, it becomes obvious that the code does not handle daylight savings time. Given that, what is the point of this code? Look at g_date_time_get_week_number and notice that it does not know that there are several different ways of numbering weeks. (Luckily that part of the code is currently unreachable.) Look at g_date_time_printf and notice that it gets 12 noon wrong! g_date_time_to_epoch fails to handle the timezone. g_date_time_new_full is interesting. It allows, say, 23:59:60 but if you create such a time and ask for its parts you get 24:00:00. Those parts cannot be fed back into g_date_time_new_full. I know I have an evil eye for this kind of thing, but this level of problems is ridiculous.
patches, as always, are welcome
(or maybe you only have the eye, but not the hand...)
Would you accept a patch to just rip this out of glib? g_date_time_printf: why does this have to use a quadratic method to walk over the utf8 string. (While assigning the unicode characters to a "char", just to ensure corruption.)
duping back to a reopened 50076, feel free to post further comments there. a few notes: (In reply to comment #0) > 9. It gets September 1752 wrong. And probably anything before that. No, it uses the proleptic Gregorian calendar. (And "September 1752" is only relevant for locales that were part of the British Empire at that time anyway. The rest of Western Europe switched much earlier, and most of Eastern Europe much later.) > Look at g_date_time_get_week_number and notice that it does not know that > there are several different ways of numbering weeks. Only one of them is interesting though. (The ISO way.) *** This bug has been marked as a duplicate of bug 50076 ***
> Would you accept a patch to just rip this out of glib? No, I would be interested in constructive patches, and less obvious disdain, if possible.