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 678620 - Gdk.Atom struct is printed as <void at 0x938bfcc>
Gdk.Atom struct is printed as <void at 0x938bfcc>
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: general
3.3.x
Other All
: Low minor
: ---
Assigned To: Nobody's working on this now (help wanted and appreciated)
Python bindings maintainers
Depends on: 687942
Blocks:
 
 
Reported: 2012-06-22 10:53 UTC by Bug flys
Modified: 2012-11-14 05:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test case to show Clipboard.wait_for_targets() results (312 bytes, text/x-python)
2012-06-22 10:53 UTC, Bug flys
  Details
test case using pygtk (258 bytes, text/x-python)
2012-06-25 14:52 UTC, Bug flys
  Details
This is a patch to gtk+ add logging to check if the atom names are ok in the c code (3.26 KB, patch)
2012-11-06 18:31 UTC, Gonzalo Odiard
none Details | Review
This patch add method gtk_clipboard_wait_for_targets_names (1.86 KB, patch)
2012-11-08 14:52 UTC, Gonzalo Odiard
reviewed Details | Review
Test using the method gtk_clipboard_wait_for_targets_names (723 bytes, text/plain)
2012-11-08 14:53 UTC, Gonzalo Odiard
  Details
Patch with the proposed solution (2.54 KB, patch)
2012-11-08 20:35 UTC, Gonzalo Odiard
none Details | Review
Python test case (659 bytes, text/x-python)
2012-11-08 20:36 UTC, Gonzalo Odiard
  Details
Patch with the proposed solution (2.54 KB, patch)
2012-11-08 21:01 UTC, Gonzalo Odiard
none Details | Review
patch, fixes marshalling of arrays of struct pointers (1.32 KB, patch)
2012-11-13 17:41 UTC, Carlos Garnacho
committed Details | Review

Description Bug flys 2012-06-22 10:53:30 UTC
Created attachment 217015 [details]
test case to show Clipboard.wait_for_targets() results

The call to Clipboard.wait_for_targets() should return a list of GdkAtom, while in the current pygobject, it returned a list of object to void pointers like <void at 0x938bfcc>. In good old pygtk, it returned a list of string which represent the atoms.


(True, [<void at 0x8a451e8>, <void at 0x8a451ec>, <void at 0x8a451f0>, <void at 0x8a451f4>, <void at 0x8a451f8>, <void at 0x8a451fc>, <void at 0x8a45200>, <void at 0x8a45204>])
Comment 1 Martin Pitt 2012-06-25 07:56:56 UTC
The string representation indeed leaves something to be desired, but these are indeed Gdk.Atom:

>>> targets = clippy.wait_for_targets()[1]
>>> targets[0]
<void at 0x322f4e0>

>>> type(targets[0])
<class 'gi.repository.Gdk.Atom'>

>>> print(targets[0].name())
None

In your example, none of the returned Atoms actually has a name. I'm not entirely sure whether this is expected or a bug somewhere.
Comment 2 Bug flys 2012-06-25 14:52:13 UTC
Created attachment 217209 [details]
test case using pygtk

The result should be a list of string, like using pygtk API:
  ('TIMESTAMP', 'TARGETS', 'MULTIPLE', 'SAVE_TARGETS', 'UTF8_STRING', 'COMPOUND_TEXT', 'TEXT', 'STRING', 'text/plain;charset=utf-8', 'text/plain')

which are the known format for the current clipboard contents.

Instead of pygobject API, a list of atom object with atom.name() all return None:
  [x.name() for x in targets[1]]
  [None, None, None, None, None, None, None, None, None]
Comment 3 Gonzalo Odiard 2012-11-06 18:21:03 UTC
(In reply to comment #1)
> The string representation indeed leaves something to be desired, but these are
> indeed Gdk.Atom:
> 
> >>> targets = clippy.wait_for_targets()[1]
> >>> targets[0]
> <void at 0x322f4e0>
> 
> >>> type(targets[0])
> <class 'gi.repository.Gdk.Atom'>
> 
> >>> print(targets[0].name())
> None
> 
> In your example, none of the returned Atoms actually has a name. I'm not
> entirely sure whether this is expected or a bug somewhere.

I have added logging to the c code to verify the Atoms names, and are ok. (Attached as add_logging_to_gtk_clipboard_wait_for_targets.diff)

Also I have tried to add "(element-type GdkAtom)" but do not have any effect.

One issue is in the "good old pygtk" this method was overriden http://git.gnome.org/browse/pygtk/tree/gtk/gtk.override#n4964

This issue is a blocker for me, I have tried all I can but without success. Any help is welcome.
Comment 4 Gonzalo Odiard 2012-11-06 18:31:44 UTC
Created attachment 228301 [details] [review]
This is a patch to gtk+ add logging to check if the atom names are ok in the c code
Comment 5 Dieter Verfaillie 2012-11-07 10:08:46 UTC
(In reply to comment #1)
> The string representation indeed leaves something to be desired, but these are
> indeed Gdk.Atom:
 
Are you sure? Because ATOM_TO_INDEX() does not seem to compute
an index that is valid for virtual_atom_array for whatever it
is that goes into gdk as the atom:
 
[dieterv@localhost ~]$ gdb --args python gdkatomtest.py
(gdb) b gdk_atom_name
(gdb) r
Breakpoint 1, gdk_atom_name (atom=0x81cd5f8) at gdkdisplaymanager.c:444
444 {
(gdb) s
445 GdkDisplayManager *manager = gdk_display_manager_get ();
(gdb)
gdk_display_manager_get () at gdkdisplaymanager.c:229
229 {
(gdb)
232 if (!manager)
(gdb)
269 }
(gdb)
gdk_atom_name (atom=0x81cd5f8) at gdkdisplaymanager.c:447
447 return GDK_DISPLAY_MANAGER_GET_CLASS (manager)->get_atom_name (manager, atom);
(gdb)
_gdk_x11_display_manager_get_atom_name (manager=0x8081e30 [GdkX11DisplayManager], atom=0x81cd5f8) at gdkproperty-x11.c:438
438 {
(gdb)
439 return g_strdup (get_atom_name (atom));
(gdb)
get_atom_name (atom=atom@entry=0x81cd5f8) at gdkproperty-x11.c:425
425 {
(gdb)
426 virtual_atom_check_init ();
(gdb)
virtual_atom_check_init () at gdkproperty-x11.c:383
383 if (!virtual_atom_hash)
(gdb)
get_atom_name (atom=atom@entry=0x81cd5f8) at gdkproperty-x11.c:428
428 if (ATOM_TO_INDEX (atom) < virtual_atom_array->len)
(gdb) p ((guint) (guint) (atom))
$1 = 136107512
(gdb) p virtual_atom_array->len
$2 = 133
 
Because of this, get_atom_name() decides to return NULL.
 
Oh, to clarify where I got the definition of ATOM_TO_INDEX from:
gdkproperty-x11.c:167 #define ATOM_TO_INDEX(atom) (GPOINTER_TO_UINT(atom))
glibconfig.h:85 #define GPOINTER_TO_UINT(p) ((guint) (guint) (p))
Comment 6 Gonzalo Odiard 2012-11-08 14:51:32 UTC
I couldn't found why the atom can't be passed to the python code. Tried many different options without success.

A posible workaround is implement a method to get a list of strings with the atom names, like the old pygtk bindings did. Now, we don't have c code in the overrided methods in the python bindings, then I have implemented a method "gtk_clipboard_wait_for_targets_names" in gtkclipboard.c
I can use this code in c, but something is happening when compile, because when try to create the gir file show a error: 

gtkclipboard.h:211: Warning: Gtk: gtk_clipboard_wait_for_targets_names: return value: Missing (element-type) annotation

when clearly the element-type annotation is present.

Attached is a patch implementing the method and a c test.

I have two questions:

1) What is wrong with the introspection annotations?

2) Is this a valid solution?
Comment 7 Gonzalo Odiard 2012-11-08 14:52:23 UTC
Created attachment 228476 [details] [review]
This patch add method gtk_clipboard_wait_for_targets_names
Comment 8 Gonzalo Odiard 2012-11-08 14:53:10 UTC
Created attachment 228478 [details]
Test using the method gtk_clipboard_wait_for_targets_names
Comment 9 Dieter Verfaillie 2012-11-08 15:02:34 UTC
(In reply to comment #6)
> 1) What is wrong with the introspection annotations?

 * Returns: (element-type utf8) (transfer container): describe something here...
Comment 10 Gonzalo Odiard 2012-11-08 15:21:36 UTC
(In reply to comment #9)
> (In reply to comment #6)
> > 1) What is wrong with the introspection annotations?
> 
>  * Returns: (element-type utf8) (transfer container): describe something
> here...

Is not that. Already tried.
Comment 11 Colin Walters 2012-11-08 19:51:09 UTC
Review of attachment 228476 [details] [review]:

::: gtk/gtkclipboard.c
@@ +1818,2 @@
 /**
+ * gtk_clipboard_wait_for_target_names:

"targets", not "target".  That's why the annotation isn't working.

@@ +1822,3 @@
+ * List of target names that are present on the clipboard.
+ * This function waits for the data to be received using the main
+ * loop, so events, timeouts, etc, may be dispatched during the wait.

How about:

Like gtk_clipboard_wait_for_targets(), but returns target names as strings.  This is provided for language bindings.

@@ +1826,3 @@
+ * Return value: (element-type utf8) (transfer container):
+ *
+ * Since: 3.0

3.8 now

@@ +1837,3 @@
+
+    gtk_clipboard_wait_for_targets (clipboard, &targets, &n_targets);
+    for (i = 0; i < n_targets; i++) {

GTK+ is GNU style - brace on its own line.

@@ +1838,3 @@
+    gtk_clipboard_wait_for_targets (clipboard, &targets, &n_targets);
+    for (i = 0; i < n_targets; i++) {
+            list = g_slist_append (list, gdk_atom_name(targets[i]));

Missing space between identifier and paren.

@@ +1839,3 @@
+    for (i = 0; i < n_targets; i++) {
+            list = g_slist_append (list, gdk_atom_name(targets[i]));
+    }

You need to g_free (targets); here.
Comment 12 Paolo Borelli 2012-11-08 20:10:37 UTC
Review of attachment 228476 [details] [review]:

::: gtk/gtkclipboard.c
@@ +1838,3 @@
+    gtk_clipboard_wait_for_targets (clipboard, &targets, &n_targets);
+    for (i = 0; i < n_targets; i++) {
+ */

Also please do not list_append in a loop, it is O(n^2):

Use 

for (..) {
  list_prepend()
}

return list_reverse()
Comment 13 Gonzalo Odiard 2012-11-08 20:35:26 UTC
Created attachment 228505 [details] [review]
Patch with the proposed solution
Comment 14 Gonzalo Odiard 2012-11-08 20:36:08 UTC
Created attachment 228506 [details]
Python test case
Comment 15 Gonzalo Odiard 2012-11-08 20:38:06 UTC
Added a new patch addressing the comments, and a test case in python. As suggested in #introspection, will change the product to do visible this issue to gtk+ devels.
Comment 16 Gonzalo Odiard 2012-11-08 21:01:50 UTC
Created attachment 228508 [details] [review]
Patch with the proposed solution
Comment 17 Martin Pitt 2012-11-13 11:36:35 UTC
So, as far as I can see there are two bugs here:

 * Gdk.Atom should print the atom name for str() instead of <void at 0xdeadbeef>.

   This would be possible with an override, but as GdkAtom has no GType, we cannot override it at the moment. It needs to be boxed, or PyGObject needs to invent something else instead.

 * The atoms returned by Gtk.Clipboard.get(Gdk.SELECTION_CLIPBOARD).wait_for_targets() are apparently bogus, as Dieter debugged in comment 5. That's why calling .name() on them does not work.
Comment 18 Gonzalo Odiard 2012-11-13 11:54:16 UTC
(In reply to comment #17)

>  * The atoms returned by
> Gtk.Clipboard.get(Gdk.SELECTION_CLIPBOARD).wait_for_targets() are apparently
> bogus, as Dieter debugged in comment 5. That's why calling .name() on them does
> not work.

The atoms are ok in the C code. Apparently, g-i is confused by GdkAtom nature and can't manage a array of GdkAtom.
Comment 19 Martin Pitt 2012-11-13 12:15:12 UTC
With http://git.gnome.org/browse/pygobject/commit/?id=671361841de797ef  Gdk.Atoms now have a proper str() and repr(), so that you can print them properly. That's part 1 of the bug.

Keeping bug open until we figure out why wait_for_targets() returns bogus atoms/addresses.
Comment 20 Carlos Garnacho 2012-11-13 17:41:19 UTC
Created attachment 228909 [details] [review]
patch, fixes marshalling of arrays of struct pointers

With this patch I get the expected results on the testcase in comment #0

(True, [Gdk.Atom<TIMESTAMP>, Gdk.Atom<TARGETS>, Gdk.Atom<MULTIPLE>, Gdk.Atom<SAVE_TARGETS>, Gdk.Atom<UTF8_STRING>, Gdk.Atom<COMPOUND_TEXT>, Gdk.Atom<TEXT>, Gdk.Atom<STRING>, Gdk.Atom<text/plain;charset=utf-8>, Gdk.Atom<text/plain>])
Comment 21 Martin Pitt 2012-11-14 05:33:55 UTC
Comment on attachment 228909 [details] [review]
patch, fixes marshalling of arrays of struct pointers

Thanks Carlos! I started debugging that in bug 687942, but this looks right. I slightly adjusted it to use the same code than in the "default:" clause and added a comment:

http://git.gnome.org/browse/pygobject/commit/?id=55070cc

I also added a test case:

http://git.gnome.org/browse/pygobject/commit/?id=fc021516