--Boundary-00=_Jwg0/HXVu9nr+aB
Content-Type: text/plain;
  charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Hi all,

Here is an updated version of this patch. It eliminates all usage of the 
icalrecur_xxx() routines which pretend to do semantically useful things since 
they seem horribly broken and impossible to read. As a result:

1. A memory leak due to not freeing the iterator has gone

2. Timezone adjustments work even if the date of interest is not in the last 
phase of the definition.

Can I please check this in?

Thanks, Shaheed

--Boundary-00=_Jwg0/HXVu9nr+aB
Content-Type: text/x-diff;
  charset="us-ascii";
  name="timezone2.diff"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment;
	filename="timezone2.diff"

? timezone.diff
? timezone2.diff
Index: icalformatimpl.cpp
===================================================================
RCS file: /home/kde/kdepim/libkcal/icalformatimpl.cpp,v
retrieving revision 1.93
diff -u -p -r1.93 icalformatimpl.cpp
--- icalformatimpl.cpp	18 Nov 2003 23:37:26 -0000	1.93
+++ icalformatimpl.cpp	6 Dec 2003 16:50:28 -0000
@@ -23,6 +23,7 @@
 #include <qstring.h>
 #include <qptrlist.h>
 #include <qfile.h>
+#include <cstdlib>
 
 #include <kdebug.h>
 #include <klocale.h>
@@ -44,6 +45,281 @@ extern "C" {
 
 using namespace KCal;
 
+namespace KCal {
+
+// This macro reads any TZID setting for an icaltime. TBD: incorporate
+// this into icalproperty_get_dtend()/icalproperty_get_dtend()?
+#define GET_TZID(p,icaltime) \
+{ \
+  icalproperty *tzp = icalproperty_get_first_parameter(p,ICAL_TZID_PARAMETER); \
+  if (tzp) { \
+    icaltime.zone = icalparameter_get_tzid(tzp); \
+  } \
+}
+
+/**
+ * A TimezonePhase represents a setting within a timezone, e.g. standard or
+ * daylight savings.
+ */
+typedef struct icaltimezonephase icaltimezonephase;
+class TimezonePhase : private icaltimezonephase {
+  public:
+    /**
+     * Contructor for a timezone phase.
+     */
+    TimezonePhase(ICalFormatImpl *parent, icalcomponent *c)
+    {
+      tzname = (const char *)0;
+      is_stdandard = 1;
+      mIsStandard = 1;
+      dtstart = icaltime_null_time();
+      offsetto = 0;
+      tzoffsetfrom = 0;
+      comment = (const char *)0;
+      rdate.time = icaltime_null_time();
+      rdate.period = icalperiodtype_null_period();
+      rrule = (const char *)0;
+      mRrule = new Recurrence((Incidence *)0);
+
+      // Now do the ical reading.  
+      icalproperty *p = icalcomponent_get_first_property(c,ICAL_ANY_PROPERTY);  
+      while (p) {
+        icalproperty_kind kind = icalproperty_isa(p);
+        switch (kind) {
+    
+          case ICAL_TZNAME_PROPERTY:
+            tzname = icalproperty_get_tzname(p);
+            break;
+    
+          case ICAL_DTSTART_PROPERTY:  
+            dtstart = icalproperty_get_dtstart(p);
+            break;
+    
+          case ICAL_TZOFFSETTO_PROPERTY:  
+            offsetto = icalproperty_get_tzoffsetto(p);
+            break;
+    
+          case ICAL_TZOFFSETFROM_PROPERTY:  
+            tzoffsetfrom = icalproperty_get_tzoffsetfrom(p);
+            break;
+    
+          case ICAL_COMMENT_PROPERTY:  
+            comment = icalproperty_get_comment(p);
+            break;
+    
+          case ICAL_RDATE_PROPERTY:  
+            rdate = icalproperty_get_rdate(p);
+            break;
+    
+          case ICAL_RRULE_PROPERTY:  
+            {
+              struct icalrecurrencetype r = icalproperty_get_rrule(p);
+
+              parent->readRecurrence(r,mRrule);
+            }
+            break;
+    
+          default:
+            kdDebug(5800) << "TimezonePhase::TimezonePhase(): Unknown property: " << kind
+                      << endl;
+            break;
+        }
+        p = icalcomponent_get_next_property(c,ICAL_ANY_PROPERTY);
+      }
+    }
+    
+    /**
+     * Destructor for a timezone phase.
+     */
+    ~TimezonePhase()
+    {
+      delete mRrule;
+    }
+
+    /**
+     * Find the nearest start time of this phase just before a given time.
+     */
+    QDateTime nearestStart(const QDateTime &t) const
+    {      
+      QDateTime tmp(QDate(dtstart.year,dtstart.month,dtstart.day), QTime(dtstart.hour,dtstart.minute,dtstart.second));     
+      // If this phase was not valid at the given time, give up.
+      if (tmp > t) {
+        kdDebug(5800) << "TimezonePhase::nearestStart(): Phase not valid" << endl;
+        return QDateTime();
+      }
+      
+      // The Recurrance class's getPreviousDateTime() logic was not designed for 
+      // start times which are not aligned with a reference time, but a little 
+      // magic is sufficient to work around that...
+      QDateTime previous = mRrule->getPreviousDateTime(tmp);
+      if (mRrule->getNextDateTime(previous) < tmp)
+        previous = mRrule->getNextDateTime(previous);
+      return previous;
+    }
+
+    /**
+     * Offset of this phase in seconds.
+     */    
+    int offset() const
+    {
+      return offsetto;
+    }
+    
+    // Hide the missnamed "is_stdandard" variable in the base class.
+    int mIsStandard;
+  
+    // Supplement the "rrule" in the base class.
+    Recurrence *mRrule;
+};
+
+/**
+ * A Timezone.
+ */
+typedef struct icaltimezonetype icaltimezonetype;
+class Timezone : private icaltimezonetype {
+  public:
+    /**
+     * Contructor for a timezone.
+     */
+    Timezone(ICalFormatImpl *parent, icalcomponent *vtimezone)
+    {
+      tzid = (const char *)0;
+      last_mod = icaltime_null_time();
+      tzurl = (const char *)0;
+      
+      // The phases list is defined to be terminated by a phase with a
+      // null name.
+      phases = (icaltimezonephase *)malloc(sizeof(*phases));
+      phases[0].tzname = (const char *)0;
+      mPhases = new QPtrList<TimezonePhase>;
+      mPhases->setAutoDelete(true);
+
+      // Now do the ical reading.  
+      icalproperty *p = icalcomponent_get_first_property(vtimezone,ICAL_ANY_PROPERTY);   
+      while (p) {
+        icalproperty_kind kind = icalproperty_isa(p);
+        switch (kind) {
+    
+          case ICAL_TZID_PROPERTY:  
+            // The timezone id is basically a unique string which is used to 
+            // identify this timezone. Note that if it begins with a "/", then it
+            // is suppsed to have some externally specified meaning, but we are
+            // just after its unique value.
+            tzid = icalproperty_get_tzid(p);
+            break;
+    
+          case ICAL_TZURL_PROPERTY:  
+            tzurl = icalproperty_get_tzurl(p);
+            break;
+    
+          default:
+            kdDebug(5800) << "Timezone::Timezone(): Unknown property: " << kind
+                      << endl;
+            break;
+        }
+        p = icalcomponent_get_next_property(vtimezone,ICAL_ANY_PROPERTY);
+      }
+      kdDebug(5800) << "---zoneId: \"" << tzid << '"' << endl;
+    
+      icalcomponent *c;
+    
+      TimezonePhase *phase;
+      
+      // Iterate through all timezones before we do anything else. That way, the
+      // information needed to interpret times in actually usefulobject is 
+      // available below.
+      c = icalcomponent_get_first_component(vtimezone,ICAL_ANY_COMPONENT);
+      while (c) {
+        icalcomponent_kind kind = icalcomponent_isa(c);
+        switch (kind) {
+    
+          case ICAL_XSTANDARD_COMPONENT:  
+            kdDebug(5800) << "---standard phase: found" << endl;
+            phase = new TimezonePhase(parent,c);
+            phase->mIsStandard = 1;
+            mPhases->append(phase);
+            break;
+    
+          case ICAL_XDAYLIGHT_COMPONENT:  
+            kdDebug(5800) << "---daylight phase: found" << endl;
+            phase = new TimezonePhase(parent,c);
+            phase->mIsStandard = 0;
+            mPhases->append(phase);
+            break;
+    
+          default:
+            kdDebug(5800) << "Timezone::Timezone(): Unknown component: " << kind
+                      << endl;
+            break;
+        }
+        c = icalcomponent_get_next_component(vtimezone,ICAL_ANY_COMPONENT);
+      }
+    }
+    
+    /**
+     * Destructor for a timezone.
+     */
+    ~Timezone()
+    {
+      delete mPhases;
+      free(phases);
+    }
+  
+    /**
+     * The string id of this timezone. Make sure we always have quotes!
+     */
+    QString id() const
+    {            
+      if (tzid[0] != '"') {
+        return QString("\"") + tzid + '"';
+      } else {
+        return tzid;
+      }
+    }
+    
+    /**
+     * Find the nearest timezone phase just before a given time.
+     */
+    const TimezonePhase *nearestStart(const QDateTime &t) const
+    {
+      unsigned i;
+      unsigned result = 0;
+      QDateTime previous;
+      QDateTime next;
+      
+      // Main loop. Find the phase with the latest start date before t.
+      for (i = 0; i < mPhases->count(); i++) {
+        next = mPhases->at(i)->nearestStart(t);
+        if (previous.isNull() || previous < next) {
+          previous = next;
+          result = i;
+        }
+      }  
+      return mPhases->at(result);
+    }
+    
+    /**
+     * Convert the given time to UTC.
+     */
+    int offset(icaltimetype t)
+    { 
+      QDateTime tmp(QDate(t.year,t.month,t.day), QTime(t.hour,t.minute,t.second));     
+      const TimezonePhase *phase = nearestStart(tmp);
+      
+      if (phase) {
+        return phase->offset();
+      } else {
+        kdError(5800) << "Timezone::offset() cannot find phase for " << tmp << endl;
+        return 0;
+      }
+    }
+    
+    // Phases we have seen.
+    QPtrList<TimezonePhase> *mPhases;
+};
+
+}
+
 const int gSecondsPerMinute = 60;
 const int gSecondsPerHour   = gSecondsPerMinute * 60;
 const int gSecondsPerDay    = gSecondsPerHour   * 24;
@@ -53,10 +329,12 @@ ICalFormatImpl::ICalFormatImpl( ICalForm
   mParent( parent ), mCalendarVersion( 0 )
 {
   mCompat = new Compat;
+  mTimezones = new QDict<class Timezone>;
 }
 
 ICalFormatImpl::~ICalFormatImpl()
 {
+  delete mTimezones;
   delete mCompat;
 }
 
@@ -107,10 +385,10 @@ icalcomponent *ICalFormatImpl::writeTodo
   if (todo->hasStartDate()) {
     icaltimetype start;
     if (todo->doesFloat()) {
-//      kdDebug(5800) << "§§ Incidence " << todo->summary() << " floats." << endl;
+//      kdDebug(5800) << " Incidence " << todo->summary() << " floats." << endl;
       start = writeICalDate(todo->dtStart().date());
     } else {
-//      kdDebug(5800) << "§§ incidence " << todo->summary() << " has time." << endl;
+//      kdDebug(5800) << " incidence " << todo->summary() << " has time." << endl;
       start = writeICalDateTime(todo->dtStart());
     }
     icalcomponent_add_property(vtodo,icalproperty_new_dtstart(start));
@@ -148,10 +426,10 @@ icalcomponent *ICalFormatImpl::writeEven
   // start time
   icaltimetype start;
   if (event->doesFloat()) {
-//    kdDebug(5800) << "§§ Incidence " << event->summary() << " floats." << endl;
+//    kdDebug(5800) << " Incidence " << event->summary() << " floats." << endl;
     start = writeICalDate(event->dtStart().date());
   } else {
-//    kdDebug(5800) << "§§ incidence " << event->summary() << " has time." << endl;
+//    kdDebug(5800) << " incidence " << event->summary() << " has time." << endl;
     start = writeICalDateTime(event->dtStart());
   }
   icalcomponent_add_property(vevent,icalproperty_new_dtstart(start));
@@ -160,11 +438,11 @@ icalcomponent *ICalFormatImpl::writeEven
     // end time
     icaltimetype end;
     if (event->doesFloat()) {
-//      kdDebug(5800) << "§§ Event " << event->summary() << " floats." << endl;
+//      kdDebug(5800) << " Event " << event->summary() << " floats." << endl;
       // +1 day because end date is non-inclusive.
       end = writeICalDate( event->dtEnd().date().addDays( 1 ) );
     } else {
-//      kdDebug(5800) << "§§ Event " << event->summary() << " has time." << endl;
+//      kdDebug(5800) << " Event " << event->summary() << " has time." << endl;
       end = writeICalDateTime(event->dtEnd());
     }
     icalcomponent_add_property(vevent,icalproperty_new_dtend(end));
@@ -240,10 +518,10 @@ icalcomponent *ICalFormatImpl::writeJour
   if (journal->dtStart().isValid()) {
     icaltimetype start;
     if (journal->doesFloat()) {
-//      kdDebug(5800) << "§§ Incidence " << event->summary() << " floats." << endl;
+//      kdDebug(5800) << " Incidence " << event->summary() << " floats." << endl;
       start = writeICalDate(journal->dtStart().date());
     } else {
-//      kdDebug(5800) << "§§ incidence " << event->summary() << " has time." << endl;
+//      kdDebug(5800) << " incidence " << event->summary() << " has time." << endl;
       start = writeICalDateTime(journal->dtStart());
     }
     icalcomponent_add_property(vjournal,icalproperty_new_dtstart(start));
@@ -781,6 +1059,16 @@ icalcomponent *ICalFormatImpl::writeAlar
   return a;
 }
 
+// Read a timezone and store it in a list where it can be accessed as needed
+// by the other readXXX() routines. Note that no writeTimezone is needed 
+// because we always store in UTC.
+void ICalFormatImpl::readTimezone(icalcomponent *vtimezone)
+{
+  Timezone *timezone = new Timezone(this, vtimezone);
+  
+  mTimezones->insert(timezone->id(), timezone);
+}
+
 Todo *ICalFormatImpl::readTodo(icalcomponent *vtodo)
 {
   Todo *todo = new Todo;
@@ -865,6 +1153,7 @@ Event *ICalFormatImpl::readEvent(icalcom
 
       case ICAL_DTEND_PROPERTY:  // start date and time
         icaltime = icalproperty_get_dtend(p);
+        GET_TZID(p,icaltime);
         if (icaltime.is_date) {
           event->setFloats( true );
           // End date is non-inclusive
@@ -984,11 +1273,13 @@ FreeBusy *ICalFormatImpl::readFreeBusy(i
 
       case ICAL_DTSTART_PROPERTY:  // start date and time
         icaltime = icalproperty_get_dtstart(p);
+        GET_TZID(p,icaltime);
         freebusy->setDtStart(readICalDateTime(icaltime));
         break;
 
       case ICAL_DTEND_PROPERTY:  // start End Date and Time
         icaltime = icalproperty_get_dtend(p);
+        GET_TZID(p,icaltime);
         freebusy->setDtEnd(readICalDateTime(icaltime));
         break;
 
@@ -1169,6 +1460,7 @@ void ICalFormatImpl::readIncidence(icalc
 
       case ICAL_DTSTART_PROPERTY:  // start date and time
         icaltime = icalproperty_get_dtstart(p);
+        GET_TZID(p,icaltime);
         if (icaltime.is_date) {
           incidence->setDtStart(QDateTime(readICalDate(icaltime),QTime(0,0,0)));
           incidence->setFloats(true);
@@ -1700,8 +1992,31 @@ QDateTime ICalFormatImpl::readICalDateTi
   kdDebug(5800) << "--- H: " << t.hour << " M: " << t.minute << " S: " << t.second
             << endl;
   kdDebug(5800) << "--- isDate: " << t.is_date << endl;
+  kdDebug(5800) << "--- isUtc: " << t.is_utc << endl;
+  kdDebug(5800) << "--- zoneId: " << t.zone << endl;
 */
 
+  // First convert the time into UTC if required.
+  if ( !t.is_utc && t.zone ) {
+    Timezone *timezone;
+    
+    // Always lookup with quotes.
+    if (t.zone[0] != '"') {
+      timezone = mTimezones->find(QString("\"") + t.zone + '"');
+    } else {
+      timezone = mTimezones->find(t.zone);
+    }
+    if (timezone) {
+      // Apply the offset, and mark the structure as UTC!
+      t.second -= timezone->offset(t);
+      t = icaltime_normalize(t);
+      t.is_utc = 1;
+    } else {
+      kdError(5800) << "ICalFormatImpl::readICalDateTime() cannot find timezone " 
+            << t.zone << endl;
+    }
+  }
+  
   if ( t.is_utc && mCompat->useTimeZoneShift() ) {
 //    kdDebug(5800) << "--- Converting time to zone '" << cal->timeZoneId() << "'." << endl;
     if (mParent->timeZoneId().isEmpty())
@@ -1895,6 +2210,16 @@ bool ICalFormatImpl::populate( Calendar 
 
   icalcomponent *c;
 
+  // Iterate through all timezones before we do anything else. That way, the
+  // information needed to interpret times in actually useful objects is 
+  // available below.
+  c = icalcomponent_get_first_component(calendar,ICAL_VTIMEZONE_COMPONENT);
+  while (c) {
+//    kdDebug(5800) << "----Timezone found" << endl;
+    readTimezone(c);
+    c = icalcomponent_get_next_component(calendar,ICAL_VTIMEZONE_COMPONENT);
+  }
+
   // Iterate through all todos
   c = icalcomponent_get_first_component(calendar,ICAL_VTODO_COMPONENT);
   while (c) {
Index: icalformatimpl.h
===================================================================
RCS file: /home/kde/kdepim/libkcal/icalformatimpl.h,v
retrieving revision 1.22
diff -u -p -r1.22 icalformatimpl.h
--- icalformatimpl.h	22 Jul 2003 18:56:46 -0000	1.22
+++ icalformatimpl.h	6 Dec 2003 16:50:28 -0000
@@ -86,9 +86,11 @@ class ICalFormatImpl {
     void writeCustomProperties(icalcomponent *parent,CustomProperties *);
     void readCustomProperties(icalcomponent *parent,CustomProperties *);
     void dumpIcalRecurrence(icalrecurrencetype);
+    void readTimezone(icalcomponent *vtimezone);
 
     ICalFormat *mParent;
     Calendar *mCalendar;
+    QDict<class Timezone> *mTimezones;
 
     QString mLoadedProductId;         // PRODID string loaded from calendar file
     int mCalendarVersion;             // determines backward compatibility mode on read

--Boundary-00=_Jwg0/HXVu9nr+aB
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
kde-pim mailing list
kde-pim@mail.kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
kde-pim home page at http://pim.kde.org/
--Boundary-00=_Jwg0/HXVu9nr+aB--