GNOME Bugzilla – Bug 553179
Gtk::Clipboard should allow use of dynamic memory?
Last modified: 2013-05-23 17:04:53 UTC
The 'ideal' example code contains a bug, in void ExampleWindow::on_button_copy() it says: //Store the copied data until it is pasted: m_ClipboardStore = strData; [...] refClipboard->set(listTargets, sigc::mem_fun(*this, &ExampleWindow::on_clipboard_get), sigc::mem_fun(*this, &ExampleWindow::on_clipboard_clear) ); However, if the clipboard was filled just before with the function, then a call to Gtk::Clipboard::set calls the 'clear' function (because it is being set again). Hence, this code would immediately erase m_ClipboardStore again. This is easy to reproduce with the example code given in http://www.gtkmm.org/docs/gtkmm-2.4/examples/book/clipboard/ideal/ Open two applications and try to copy and paste things repeatedly; all you'll get is nothing. Moving the line 'm_ClipboardStore = strData' *below* refClipboard->set(..) fixes the application.
Thanks. Could you provide an actual patch (against the gtkmm-documentation module in svn) that you have tested, please.
After some discussion on #GTK+, Michael Natterer pointed me to gtktextbuffer.c in the GTK+ source code as an example of how to use gtk_clipboard_set_with_data. The correct GTK+ way seems to be to allocate dynamic memory for the copied clipboard contents (to be stored until the get_func is called) and free that in the clear_func by *passing the pointer to the allocated data to gtk_clipboard_set_with_data*. This pointer will then be passed to the clear_func, so that it can free it. The same pointer is also passed to the get_func, so that it can use it. In the tutorial this goes wrong because the gtkmm API doesn't ALLOW to pass a pointer to gtk_clipboard_set_with_data. Gtk::Clipboard::set already uses this pointer for a dynamically allocated SignalProxy_GetClear object. Imho, this means there is something wrong with how gtkmm wraps gtk_clipboard_set_with_data. Correct would be to allow passing a dynamically allocated object and passing that pointer back when calling the clear_func call back function.
We can pass a pointer (or some other identifier) by using sigc::bind() around the two callback slots. That's easy. But I'm trying to think of a way to do this without just passing a pointer to memory that is not stored anywhere else. I know that would work but it doesn't seem very pretty.
Do you have a suggestion?
Maybe change Gtk::Clipboard::set and replace the slot_clear with a Gtk::RefPtr<Gtk::ClipboardContent<T> >, which then would also be passed back to the slot_get user function, where the user would be able to access the 'T' object. The RefPtr would be kept around until after the clipboard is cleared (the point where usually slot_clear is called). At that point the RefPtr is destructed and therefore the dynamically allocated copy of T is destructed (you could view ~T as the old slot_clear function). Sorry.. I don't have time finish this comment...
Created attachment 172366 [details] [review] Patch for clipboard/ideal example This bug report started by describing a simple bug in the gtkmm example program clipboard/ideal. Then the ambition seems to have increased to such an extent that no one has time to finish the discussion, and no one cares much about the original very easily corrected bug any more. Let's get back to the problems in the clipboard/ideal example that make it not quite entitled to the title 'ideal'. I have found two problems: 1. The bug that Carlo Wood has found and described. He has also described a simple solution. 2. Another drawback with this example is that it does not detect when the contents of the clipboard has changed. Therefore it does not update the status of the Paste button when another program makes changes to the clipboard. A solution to problem 2 is to connect a signal handler to the signal owner_change. I'm not quite sure that this signal is always emitted when the contents of the clipboard changes, but probably it is. Anyway it's the method used by e.g. the editor gedit. And owner_change is the only signal emitted by GtkClipboard and Gtk::Clipboard, so probably there is no better way. Two years ago Murray Cumming asked for an svn patch. I suppose a Git patch is more appropriate now, so I've made one. Since this is the first time I attach a patch to a bug report, I'm not certain that it's as complete as it should be. E.g. am I supposed to add a few lines to the ChangeLog and NEWS files? I have not done that. I've not tested my changes with gtkmm-3.0, but with gtkmm-2.4 v2.20.3 on Ubuntu 10.10.
Well, you should generally patch the ChangeLog for this project, yes, but your patch looks perfectly clean otherwise. So, do you think this patch completely resolves this bug? That would be cool.
Yes, the patch in comment 6 resolves the bug mentioned in the bug description. It implements the solution described in the last two lines of the description, and it also introduces another improvement in the clipboard/ideal example (item 2 in comment 6). But I've completely ignored the discussion in comments 2-5. That's a question of changing the API of class Gtk::Clipboard. I don't quite see the need for such a change, but if anyone else does, I suppose it should be handled in a separate bug report. Is my patch good enough for you, or shall I make a new one? File gtkmm-tutorial-in.xml has been changed several times in the Git repository after I cloned it. Does that make it difficult for you to use my patch? (Git is fairly new to me, and I don't know have clever it is, when it applies a patch.)
(In reply to comment #8) > Yes, the patch in comment 6 resolves the bug mentioned in the bug description. > It implements the solution described in the last two lines of the description, > and it also introduces another improvement in the clipboard/ideal example > (item 2 in comment 6). OK. Great. Pushed to the gtkmm-2-22 and master branches of gtkmm-documentation. Thanks. > But I've completely ignored the discussion in comments > 2-5. That's a question of changing the API of class Gtk::Clipboard. I don't > quite see the need for such a change, but if anyone else does, I suppose it > should be handled in a separate bug report. I'll rename this one and keep it open.
[Setting QA Contact of all open gtkmm/documentation tickets to 'gtkmm-forge@' so people interested in following reports' changes via the users watchlist can still follow if the assignee is changed to a real person.]
(In response to comment #2) > Imho, this means there is something wrong with how gtkmm wraps > gtk_clipboard_set_with_data. Correct would be to allow passing a > dynamically allocated object and passing that pointer back when calling > the clear_func call back function. I disagree. gtk_clipboard_set_with_data() is correctly wrapped in gtkmm. What is right in gtk+/C is not necessarily the best way in gtkmm/C++. When callback functions that take a void* user_data are wrapped in gtkmm, it's common practice to use that user_data for a pointer to a sigc::slot, or a pointer to another object that contains a sigc::slot. This difference between gtk+ and gtkmm is discussed in a section in the gtkmm tutorial, https://developer.gnome.org/gtkmm-tutorial/stable/sec-binding-extra-arguments.html.en See especially the last paragraph in that section. If you really want to transfer a pointer to the get_func and/or clear_func, you can easily do it with std::bind(), like Murray says in comment 3. But IMHO the method used in the clipboard/ideal example is better. I close this bug. Feel free to reopen, if you still want Gtk::Clipboard::set() changed. I choose resolution FIXED, because the original bug, described in the bug description, was fixed 2.5 years ago. As for the discussion in comment 2, I consider it NOTABUG.
(In reply to comment #11) > you can easily do it with std::bind(), Shall be sigc::bind().