FW: apparent bug in 7/26/06 version of 'tzload()'
David Lawless is not on the time zone mailing list; direct replies appropriately. -----Original Message----- From: David Lawless [mailto:lawless@spamcop.net] Sent: Thursday, July 27, 2006 2:04 PM To: tz@lecserver.nci.nih.gov Subject: apparent bug in 7/26/06 version of 'tzload()' While experimenting with 'localhost.c' I discovered a bug in 'localtime.c' in 'tzload()'. The last few lines setup two flags 'goback' and 'goahead'. This code executes some invalid negative-offset array dereferences when the number of points in the array is too small. RHEL4.3 (CentOS 4.3) has zone files with fewer than 800 years in them and provokes this. I believe the attached patch corrects the problem. However I don't understand the purpose of the code and may have got it wrong. Please CC my e-mail with any replies as I'm not on the mailing list. Regards, David Lawless
David's analysis and fix look good to me; I plan to incorporate the fix in the next time zone bundle. Now if only C had an &&= operator... --ado -----Original Message----- From: Olson, Arthur David (NIH/NCI) [E] Sent: Thursday, July 27, 2006 2:20 PM To: tz@lecserver.nci.nih.gov Cc: lawless@spamcop.net Subject: FW: apparent bug in 7/26/06 version of 'tzload()' David Lawless is not on the time zone mailing list; direct replies appropriately. -----Original Message----- From: David Lawless [mailto:lawless@spamcop.net] Sent: Thursday, July 27, 2006 2:04 PM To: tz@lecserver.nci.nih.gov Subject: apparent bug in 7/26/06 version of 'tzload()' While experimenting with 'localhost.c' I discovered a bug in 'localtime.c' in 'tzload()'. The last few lines setup two flags 'goback' and 'goahead'. This code executes some invalid negative-offset array dereferences when the number of points in the array is too small. RHEL4.3 (CentOS 4.3) has zone files with fewer than 800 years in them and provokes this. I believe the attached patch corrects the problem. However I don't understand the purpose of the code and may have got it wrong. Please CC my e-mail with any replies as I'm not on the mailing list. Regards, David Lawless
Olson, Arthur David (NIH/NCI) [E] said:
David's analysis and fix look good to me; I plan to incorporate the fix in the next time zone bundle.
Now if only C had an &&= operator...
&=!! -- Clive D.W. Feather | Work: <clive@demon.net> | Tel: +44 20 8495 6138 Internet Expert | Home: <clive@davros.org> | Fax: +44 870 051 9937 Demon Internet | WWW: http://www.davros.org | Mobile: +44 7973 377646 THUS plc | |
Date: Thu, 27 Jul 2006 14:22:24 -0400 From: "Olson, Arthur David \(NIH/NCI\) [E]" <olsona@dc37a.nci.nih.gov> Message-ID: <B410D30A78C6404C9DABEA31B54A2813997ADE@nihcesmlbx10.nih.gov> | Now if only C had an &&= operator... Sometimes that would be useful, but it turns out, it would not be needed here. What's more, now those inappropriate &= can be removed as well - the "if" that is added assures us that sp->goback (and sp->goahead of course) are true before those expressions are evaluated. And of course, true && x is identical to x, so all you need in those expressions is a simple assignment now. I'd also add extra parentheses in the "if". Well, actually, I probably wouldn't, but with the absurd pickiness (programmer protection, even when neither wanted nor desired) of C compilers these days, it is probably a good idea to make it be if ((sp->goback = sp->goahead = (sp->timecnt > i))) { And even perhaps insert a != 0 test, just to show that it really is intended. Or, if you were willing to make it be a little different style, write it as ... sp->goback = sp->goahead = FALSE /* or 0 */ ; if (sp->timecnt > i) { sp->goback = sp->types[i] == sp->types[0] && differ_by_repeat(sp->ats[i], sp->ats[0]); sp->goahead = sp->types[sp->timecnt - 1] == sp->types[sp->timecnt - 1 - i] && differ_by_repeat(sp->ats[sp->timecnt - 1], sp->ats[sp->timecnt - 1 - i]); } kre
participants (3)
-
Clive D.W. Feather -
Olson, Arthur David (NIH/NCI) [E] -
Robert Elz