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 786528 - Please make the output deterministic
Please make the output deterministic
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Input Methods
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2017-08-19 22:56 UTC by Chris Lamb
Modified: 2018-01-22 20:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-gtk-queryimmodules.c-Make-the-output-deterministic.patch (1.57 KB, patch)
2017-08-19 22:56 UTC, Chris Lamb
none Details | Review
v2 of 0001-gtk-queryimmodules.c-Make-the-output-deterministic.patch (1.59 KB, patch)
2017-08-25 07:15 UTC, Chris Lamb
none Details | Review
0001-gtk-queryimmodules.c-Make-the-output-deterministic.patch (1.59 KB, patch)
2017-08-25 15:25 UTC, Chris Lamb
none Details | Review

Description Chris Lamb 2017-08-19 22:56:26 UTC
Created attachment 357999 [details] [review]
0001-gtk-queryimmodules.c-Make-the-output-deterministic.patch

Whilst working on the Reproducible Builds effort [0], we noticed that
queryimmodules generates non-reproducible output as it iterates over the
filesystem without sorting.

Patch attached.

 [0] https://reproducible-builds.org/
Comment 1 Matthias Clasen 2017-08-22 11:13:08 UTC
Review of attachment 357999 [details] [review]:

Thanks for the patch! Only some minor fixes needed

::: gtk/queryimmodules.c
@@ +196,3 @@
               {
                 const char *dent;
+                GSList *list = NULL, *iterator = NULL;

Not that it matters much, but if you want to sort, using a GList or an array would be better.

@@ +208,3 @@
                   }
 
+                g_slist_free (list);

You are leaking the duplicated strings here.
Comment 2 Chris Lamb 2017-08-25 07:14:25 UTC
Hi Matthias,

Thanks for the review. Attaching an updated patch that moves to a GList (doubly-linked) and uses g_list_free_full to free the strdup'd contents of said list.
Comment 3 Chris Lamb 2017-08-25 07:15:11 UTC
Created attachment 358384 [details] [review]
v2 of 0001-gtk-queryimmodules.c-Make-the-output-deterministic.patch

v2
Comment 4 Matthias Clasen 2017-08-25 15:13:58 UTC
Review of attachment 358384 [details] [review]:

With that fix, it looks fine to commit

::: gtk/queryimmodules.c
@@ +208,3 @@
                   }
 
+                g_list_free_full (list, g_object_unref);

This needs to be g_free - your list contains dup'ed strings
Comment 5 Chris Lamb 2017-08-25 15:25:47 UTC
Created attachment 358411 [details] [review]
0001-gtk-queryimmodules.c-Make-the-output-deterministic.patch

Updated patch attached.
Comment 6 Chris Lamb 2017-09-13 18:47:03 UTC
NB. This patch applies to both the latest GTK2 and GTK3, with only the offset differing. Please apply to both branches.

In Debian, these have two separate bugs:

https://bugs.debian.org/872729 (GTK2)
https://bugs.debian.org/875700 (GTK3)
Comment 7 Chris Lamb 2018-01-17 06:52:51 UTC
I just got a mail that this was fix.ed Was this merged under GTK2 or GTK3? :)
Comment 8 Daniel Boles 2018-01-22 20:06:46 UTC
only 3, but I've pushed it to 2 now