i-cal-attach.xml, icalattach.c - improve memory handling

The problem here is the change of icalattach structure, or
better due to change of icalattach_new_from_data(). The
previous versions ignored the 'free' function and always made
a copy of the passed-in data, while libical 3.0.7 doesn't do
the copy and uses the 'free' function. The libical-glib
wrapper for this function makes a copy when no 'free'
function is passed in, which mimics the previous behavior.

Could the icalattach_new_from_data() itself be changed, to
have a similar code as its libical-glib counterpart? That is,
when no 'free' function is passed in, then copy the 'data'
and set the 'free' function on itself? That would preserve
previous behavior.

The attached is a fix for libical doing just that,
which preserves backward compatibility.

2 files changed, 16 insertions, 11 deletions

diff --git a/src/libical-glib/api/i-cal-attach.xml b/src/libical-glib/api/i-cal-attach.xml
index 1402aa06..69108e48 100644
--- a/src/libical-glib/api/i-cal-attach.xml
+++ b/src/libical-glib/api/i-cal-attach.xml
@@ -19,12 +19,6 @@
         <returns type="ICalAttach *" annotation="transfer full" comment="The newly created #ICalAttach from the @url" />
         <comment xml:space="preserve">Create a new #ICalAttach from the url</comment>
     </method>
-    <declaration position="body">
-static void icalglib_free_attach_data(gpointer data, gpointer user_data)
-{
-    _unused(user_data);
-    g_free (data);
-}</declaration>
     <method name="i_cal_attach_new_from_data" corresponds="CUSTOM" kind="constructor" since="1.0">
         <parameter type="const gchar *" name="data" comment="The data used to create the #ICalAttach"/>
         <parameter type="GFunc" name="free_fn" translator="(icalattach_free_fn_t)" annotation="scope call, allow-none" comment="The function used to free the data when the create #ICalAttach is detroyed."/>
@@ -33,11 +27,6 @@ static void icalglib_free_attach_data(gpointer data, gpointer user_data)
         <comment xml:space="preserve">Create a new #ICalAttach from the data.</comment>
         <custom>    g_return_val_if_fail (data != NULL, NULL);
 
-    if (!free_fn) {
-        data = g_strdup(data);
-        free_fn = icalglib_free_attach_data;
-    }
-
     return i_cal_attach_new_full (icalattach_new_from_data (data, (icalattach_free_fn_t) (free_fn), free_fn_data), NULL);</custom>
     </method>
     <declaration position="body">
diff --git a/src/libical/icalattach.c b/src/libical/icalattach.c
index d01153af..423cc27e 100644
--- a/src/libical/icalattach.c
+++ b/src/libical/icalattach.c
@@ -51,6 +51,12 @@ icalattach *icalattach_new_from_url(const char *url)
     return attach;
 }
 
+static void attach_data_free(char *data, void *free_fn_data)
+{
+    _unused(free_fn_data);
+    free(data);
+}
+
 icalattach *icalattach_new_from_data(const char *data, icalattach_free_fn_t free_fn,
                                      void *free_fn_data)
 {
@@ -63,6 +69,16 @@ icalattach *icalattach_new_from_data(const char *data, icalattach_free_fn_t free
         return NULL;
     }
 
+    if (!free_fn) {
+        data = strdup(data);
+        if (!data) {
+            free(attach);
+            errno = ENOMEM;
+            return NULL;
+        }
+        free_fn = attach_data_free;
+    }
+
     attach->refcount = 1;
     attach->is_url = 0;
     attach->u.data.data = (char *) data;