[Kde-pim] [PATCH] Fix RecurrenceRule::getNextDate()

[David Jarvie]

On Thursday 15 Feb 2007 15:40, Will Stephenson wrote:
>On Thursday 15 February 2007 00:55, David Jarvie wrote:
>> I don't have time to look at it properly tonight (I'll do so tomorrow), but
>> I tend to feel that the fix might be better applied further up the call
>> chain. I think that in RecurrenceRule::getNextDate(), there could be a test
>> near the beginning:
>>
>> if ( preDate 
>>   preDate = startDt().addSecs(-1);
>>
>> I know that preDate is const, so this is really pseudo-code, but something
>> equivalent might work.
>>
>> There is probably no harm in applying your fix in case of other bugs, but I
>> think that it can probably be caught before it gets there.
>
>I've tried this approach as well - it seems to work, see diff.
>
>I thought there might be a danger that if startDt falls on the first second of 
>a 'period' that the code in getNextValidDateInterval() that normalises start 
>and toDate to the start of their 'period' (eg week), the preDate adjusted as 
>you suggest would end up in the previous 'period' and we get a value of -1 
>for periods.  This does in fact happen, but the check at line 1136:
>  while ( dtit != dts.end() && (*dtit) 
>rejects the spurious date returned by getDatesForInterval() as it is now 
>before adjustedPreDate.  The calendar in testalarms2.diff and the modified 
>testalarms program check this case.
>
>This would also be the case if we had Secondly recurrence as *ditit == 
>adjustedPreDate, and i think this is the most extreme corner case at this 
>point.
>
>What do you think is the better solution?  Do you think the fix is safe?  I 
>would really like to close this bug and move on but I am afraid of creating 
>more bugs.

I'm inclined to think that both solutions should probably be applied. I'd
prefer to see the early date caught as soon as possible, in getNexDate()
- this follows the general way of doing things in recurrence handling code.
But putting in checks for invalid period counts in getNextValidDateInterval()
seems like a sensible precaution anyway, in case any other circumstances
cause a similar error. I can't see any drawbacks to either change.

--
David Jarvie.
KAlarm author & maintainer.
http://www.astrojar.org.uk/linux/kalarm.html

_______________________________________________
kde-pim mailing list
kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
kde-pim home page at http://pim.kde.org/