"Robert E. Bruccoleri" <bruc@stone.congenomics.com> writes:
>> No, the code is correct, although no doubt too clever by half :-(
> How can it be correct? If the assertion checking is turned off, then
> saveState.currBucket will not be changed, but if assertion checking is
> on, it will be set to NULL. The only way that it would make no
> difference would be if the saveState.currBucket were NULL to begin
> with, but then, why make the assignment?
If assertion checking is off (and the code is otherwise correct) then
there's no need to reset saveState.currBucket to NULL, or so at least
I interpret the author's intent. Notice that an == test would be
completely redundant with the first part of the Assert, since the
local currBucket was just assigned from saveState.currBucket. What
is really being accomplished here is equivalent to Assert(currBucket);
#ifdef USE_ASSERT_CHECKING saveState.currBucket = NULL;
#endif
However, it's a tad silly to confuse readers this much in order to save
one lousy store instruction, so I'm inclined to change it to Assert(currBucket); saveState.currBucket = NULL;
IIRC, you're not the first to look at that code and think it's wrong.
>>> All the regression tests pass except for tests involving Savings Time
>>> which are off by one hour. --Bob
>>
>> Details? If you ran it today then the DST-boundary problems shouldn't
>> be there anymore.
> Here are the diffs:
It looks like your files match the solaris-1947 variants; would you
confirm that? If so, please send a patch with a resultmap addition
that matches your platform. There is already an entry
horology/.*-irix6=horology-no-DST-before-1970
but it looks like that's not triggering on your system.
regards, tom lane
