icalrecur: Let iterator reference the passed rule rather than copying…

… it.
minichma · Sep 29, 2024 · 1ff4e92 · 1ff4e92
1 parent d417fef
commit 1ff4e92
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 18 deletions.
diff --git a/src/libical/icalrecur.c b/src/libical/icalrecur.c
@@ -1148,7 +1148,12 @@ struct icalrecur_iterator_impl {
     short by_indices[ICAL_BY_NUM_PARTS];
     short orig_data[ICAL_BY_NUM_PARTS]; /**< 1 if there was data in the byrule */
 
-    icalrecurrence_by_data *by_ptrs[ICAL_BY_NUM_PARTS]; /**< Pointers into the by_* array elements of the rule */
+    const icalrecurrence_by_data *by_ptrs[ICAL_BY_NUM_PARTS]; /**< Pointers into the by_* array elements of the rule */
+
+    // Static buffers for BY values that need to be modified by the iterator, so we don't modify the rule.
+    // We have one value for each BY rule.
+    short by_buffer_values[ICAL_BY_NUM_PARTS];
+    icalrecurrence_by_data by_buffers[ICAL_BY_NUM_PARTS];
 };
 
 static void daysmask_clearall(unsigned long mask[])
@@ -1178,6 +1183,16 @@ static int has_by_data(icalrecur_iterator *impl, icalrecurrencetype_byrule byrul
     return (impl->orig_data[byrule] == 1);
 }
 
+static void recur_iterator_set_static_single_by_value(icalrecur_iterator *impl,
+                                              icalrecurrencetype_byrule byrule, short value)
+{
+    icalrecurrence_by_data *by = &impl->by_buffers[byrule];
+    by->size = 1;
+    by->data = &impl->by_buffer_values[byrule];
+    by->data[0] = value;
+    impl->by_ptrs[byrule] = by;
+}
+
 static void setup_defaults(icalrecur_iterator *impl,
                            icalrecurrencetype_byrule byrule, int deftime)
 {
@@ -1187,7 +1202,7 @@ static void setup_defaults(icalrecur_iterator *impl,
         /* Re-write the BY rule arrays with data from the DTSTART time so
            we don't have to explicitly deal with DTSTART */
         if (impl->by_ptrs[byrule]->size == 0) {
-            icalrecur_set_single_by(impl->by_ptrs[byrule], (short)deftime);
+            recur_iterator_set_static_single_by_value(impl, byrule, (short)deftime);
         }
     }
 }
@@ -1568,7 +1583,7 @@ static int validate_byrule(icalrecur_iterator *impl,
 {
     if (has_by_data(impl, byrule)) {
         UErrorCode status = U_ZERO_ERROR;
-        icalrecurrence_by_data *by_ptr = impl->by_ptrs[byrule];
+        const icalrecurrence_by_data *by_ptr = impl->by_ptrs[byrule];
         short max =
             (short)ucal_getLimit(impl->rscale, field, UCAL_MAXIMUM, &status);
         short idx;
@@ -2139,11 +2154,9 @@ icalrecur_iterator *icalrecur_iterator_new(struct icalrecurrencetype *rule,
     memset(impl, 0, sizeof(icalrecur_iterator));
 
     impl->dtstart = dtstart;
-    impl->rule = icalrecurrencetype_clone(rule);
-    if (!impl->rule) {
-        icalrecur_iterator_free(impl);
-        return 0;
-    }
+
+    icalrecurrencetype_ref(rule);
+    impl->rule = rule;
 
     impl->iend = icaltime_null_time();
 
@@ -2370,13 +2383,12 @@ static int next_unit(icalrecur_iterator *impl,
             end_of_data = 1;
         }
 
-        if (impl->by_ptrs[by_unit]->size <= impl->by_indices[by_unit]) {
-            if (icalrecur_resize_by(impl->by_ptrs[by_unit], impl->by_indices[by_unit] + 1))
-                return 0;
+        if (impl->by_indices[by_unit] < impl->by_ptrs[by_unit]->size) {
+            set_unit(impl, impl->by_ptrs[by_unit]->data[impl->by_indices[by_unit]]);
+        } else {
+            icalerror_set_errno(ICAL_INTERNAL_ERROR);
         }
 
-        set_unit(impl, impl->by_ptrs[by_unit]->data[impl->by_indices[by_unit]]);
-
     } else if (!has_by_unit && this_frequency) {
         /* Compute the next value from the last time and the freq interval */
         increment_unit(impl, impl->rule->interval);
@@ -3562,10 +3574,7 @@ static int __iterator_set_start(icalrecur_iterator *impl, icaltimetype start)
         if (impl->by_ptrs[ICAL_BY_DAY]->size <= 0) {
             /* Weekly recurrences with no ICAL_BY_DAY data should occur on the
                same day of the week as the start time . */
-            if (icalrecur_set_single_by(impl->by_ptrs[ICAL_BY_DAY], (short)get_day_of_week(impl))) {
-                icalerror_set_errno(ICAL_NEWFAILED_ERROR);
-                return 0;
-            }
+            recur_iterator_set_static_single_by_value(impl, ICAL_BY_DAY, (short)get_day_of_week(impl));
         } else {
             adjust_to_byday(impl);
 

diff --git a/src/libical/icalrecur.h b/src/libical/icalrecur.h
@@ -321,7 +321,11 @@ LIBICAL_ICAL_EXPORT char *icalrecurrencetype_as_string_r(struct icalrecurrencety
 
 typedef struct icalrecur_iterator_impl icalrecur_iterator;
 
-/** Creates a new recurrence rule iterator, starting at DTSTART. */
+/** Creates a new recurrence rule iterator, starting at DTSTART.
+ * 
+ * NOTE: The new iterator keeps a reference to the passed rule. It must not be modified as long as
+ * the iterator is in use.
+ */
 LIBICAL_ICAL_EXPORT icalrecur_iterator *icalrecur_iterator_new(struct icalrecurrencetype *rule,
                                                                struct icaltimetype dtstart);