GNOME Bugzilla – Bug 624197
[patch] port to GtkApplication
Last modified: 2010-11-18 10:08:27 UTC
Created attachment 165759 [details] [review] 0001-port-to-GtkApplication.patch The attached patch ports devhelp from unique to GtkApplication to provide the same functionality.
Review of attachment 165759 [details] [review]: Thanks for working on this; here is a quick review of things I could easily spot. ::: configure.ac @@ -66,3 @@ gtk+-3.0 webkitgtk-3.0 - unique-3.0 Since you are now using methods from a newer glib you should add an explicit check for glib-2.0 >= 2.25.11 ::: src/dh-main.c @@ +227,3 @@ + for (i = 0; i < G_N_ELEMENTS (commands); i++) { + g_application_add_action (application, commands[i].name, commands[i].name); + } You could add a new member to commands, to hold a description, so the description is not just the command name. @@ +244,3 @@ + argv_bytes = (guint8 *) argv[i]; + g_variant_builder_add_value (&builder, + g_variant_new_byte_array (argv_bytes, -1)); glib d9e90c38 (July 7th) removed g_variant_new_byte_array. @@ +333,3 @@ + "argv", variant_from_argv (argc, argv), + "default-quit", FALSE, + NULL); And I think your variant_from_argv could be replaced by a call to g_variant_new_bytestring_array.
(In reply to comment #1) > Review of attachment 165759 [details] [review]: > > Thanks for working on this; here is a quick review of things I could easily > spot. > > ::: configure.ac > @@ -66,3 @@ > gtk+-3.0 > webkitgtk-3.0 > - unique-3.0 > > Since you are now using methods from a newer glib you should add an explicit > check for glib-2.0 >= 2.25.11 Unstaged that hunk accidentally it seems. > ::: src/dh-main.c > @@ +227,3 @@ > + for (i = 0; i < G_N_ELEMENTS (commands); i++) { > + g_application_add_action (application, commands[i].name, > commands[i].name); > + } > > You could add a new member to commands, to hold a description, so the > description is not just the command name. Sounds like a good idea. Done. > @@ +244,3 @@ > + argv_bytes = (guint8 *) argv[i]; > + g_variant_builder_add_value (&builder, > + g_variant_new_byte_array (argv_bytes, -1)); > > glib d9e90c38 (July 7th) removed g_variant_new_byte_array. > > @@ +333,3 @@ > + "argv", variant_from_argv (argc, argv), > + "default-quit", FALSE, > + NULL); > > And I think your variant_from_argv could be replaced by a call to > g_variant_new_bytestring_array. These changes are part of 2.25.11, which was released after I wrote the patch. However, since it is out now, so, updated.
Created attachment 166101 [details] [review] 0001-port-to-GtkApplication.patch
Thanks for your quick reaction; I get this segfault when I have a devhelp instance started and send a command to it (running "devhelp -q" in another shell, for example). Can you reproduce it? Program received signal SIGSEGV, Segmentation fault. __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31 31 ../sysdeps/x86_64/multiarch/../strlen.S: Aucun fichier ou dossier de ce type. in ../sysdeps/x86_64/multiarch/../strlen.S (gdb) bt
+ Trace 222889
(In reply to comment #4) > Thanks for your quick reaction; I get this segfault when I have a devhelp > instance started and send a command to it (running "devhelp -q" in another > shell, for example). > > Can you reproduce it? No, I cant reproduce it. I have a feeling that using GApplication instead of GtkApplication will prevent this (had a similar issue with totem). Ive tried multiple times, all the commands actually work for me (tried replacing the installed version with the patched version, still nothing, inside or outside of gdb). I wonder if there is a distro-specific patch that is causing this disparity in behaviour? > Program received signal SIGSEGV, Segmentation fault. > __strlen_sse2 () at ../sysdeps/x86_64/multiarch/../strlen.S:31 > 31 ../sysdeps/x86_64/multiarch/../strlen.S: Aucun fichier ou dossier de ce > type. > in ../sysdeps/x86_64/multiarch/../strlen.S > > (gdb) bt >
Created attachment 166894 [details] [review] 0001-port-to-GApplication.patch Can you please try this patch? It simply uses GApplication rather than GtkApplication.
Thanks, it works fine; I pushed it to both master and the new gnome-2-32 branch.
GApplication API changed in GLib commit http://git.gnome.org/browse/glib/commit/?id=a7923a4aa3ff6d67672c6c69cd1b7d70fba9c57d So the pushed patch is now preventing a proper build of Devhelp, see bug 635162