After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 553179 - Gtk::Clipboard should allow use of dynamic memory?
Gtk::Clipboard should allow use of dynamic memory?
Status: RESOLVED FIXED
Product: gtkmm
Classification: Bindings
Component: documentation
2.14.x
Other All
: Normal normal
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2008-09-21 22:02 UTC by Carlo Wood
Modified: 2013-05-23 17:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch for clipboard/ideal example (3.92 KB, patch)
2010-10-14 16:36 UTC, Kjell Ahlstedt
committed Details | Review

Description Carlo Wood 2008-09-21 22:02:07 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.
Comment 1 Murray Cumming 2008-09-23 11:49:03 UTC
Thanks. Could you provide an actual patch (against the gtkmm-documentation module in svn) that you have tested, please.
Comment 2 Carlo Wood 2008-09-23 15:40:11 UTC
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.
Comment 3 Murray Cumming 2008-09-26 11:35:49 UTC
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.
Comment 4 Murray Cumming 2008-10-20 11:22:32 UTC
Do you have a suggestion?
Comment 5 Carlo Wood 2008-10-20 16:27:35 UTC
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...
Comment 6 Kjell Ahlstedt 2010-10-14 16:36:06 UTC
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.
Comment 7 Murray Cumming 2010-11-17 13:00:31 UTC
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.
Comment 8 Kjell Ahlstedt 2010-11-18 09:21:43 UTC
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.)
Comment 9 Murray Cumming 2010-11-18 11:23:33 UTC
(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.
Comment 10 André Klapper 2012-02-15 10:11:00 UTC
[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.]
Comment 11 Kjell Ahlstedt 2013-05-23 09:16:31 UTC
(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.
Comment 12 Kjell Ahlstedt 2013-05-23 17:04:53 UTC
(In reply to comment #11)
> you can easily do it with std::bind(),

Shall be sigc::bind().