clipmenud: Ensure consistent memory management for clipboard text

Previously, clipboard text was sometimes freed with XFree() and
sometimes with free(), potentially causing double frees when using the
INCR protocol for large clippings.

Convert the XGetWindowProperty()-allocated memory to a
malloc()-allocated buffer immediately in get_clipboard_text(), and
always free it with free().

Fixes #241.

src/clipmenud.c

@@ -85,7 +85,20 @@ static char *get_clipboard_text(Atom clip_atom) {
         return NULL;
     }
 
-    return (char *)cur_text;
+    // Usually the lifecycle here would be ended by XFree(), but we need
+    // consistency with INCR handling, which uses malloc(), so we can always
+    // use free(). As such, move to the malloc() pool.
+    if (!cur_text) {
+        return NULL;
+    }
+
+    char *m_cur_text = malloc(nitems + 1);
+    expect(m_cur_text);
+    memcpy(m_cur_text, cur_text, nitems);
+    m_cur_text[nitems] = '\0';
+    XFree(cur_text);
+
+    return m_cur_text;
 }
 
 /**
@@ -243,7 +256,7 @@ static uint64_t store_clip(char *text) {
     }
 
     if (last_text) {
-        XFree(last_text);
+        free(last_text);
     }
     last_text = text;
     last_text_time = current_time;
@@ -263,7 +276,7 @@ static void incr_receive_finish(struct incr_transfer *it) {
     }
 
     it_dbg(it, "Finished (bytes buffered: %zu)\n", it->data_size);
-    _drop_(free) char *text = malloc(it->data_size + 1);
+    char *text = malloc(it->data_size + 1);
     expect(text);
     memcpy(text, it->data, it->data_size);
     text[it->data_size] = '\0';
@@ -280,6 +293,7 @@ static void incr_receive_finish(struct incr_transfer *it) {
         }
     } else {
         it_dbg(it, "Clipboard text is whitespace only, ignoring\n");
+        free(text);
     }
 
     free(it->data);
@@ -423,7 +437,7 @@ static int handle_property_notify(const XPropertyEvent *pe) {
             }
         } else {
             dbg("Clipboard text is whitespace only, ignoring\n");
-            XFree(text);
+            free(text);
         }
     }
 

tests/x_integration_tests

@@ -195,6 +195,25 @@ len_after=$(xsel -po | wc -c)
 # Make sure we got the whole thing
 (( len_before == len_after ))
 
+# Issue #241. Put a large clipboard selection via INCR, then put something else
+# small immediately after.
+xsel -bc
+settle
+check_nr_clips 5
+
+# Large selection (INCR)
+printf '%.0sa' {1..4001} | xsel -b
+long_settle
+
+check_nr_clips 6
+
+# After large selection is stored, put something small that's a possible #
+# partial (non-INCR)
+printf a | xsel -b
+settle
+
+check_nr_clips 6
+
 if (( _UNSHARED )); then
     umount -l /tmp
 fi