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 777272 - archive: use GHashTable for the entries
archive: use GHashTable for the entries
Status: RESOLVED FIXED
Product: libgxps
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: libgxps maintainers
libgxps maintainers
Depends on:
Blocks:
 
 
Reported: 2017-01-15 11:45 UTC by Paolo Borelli
Modified: 2017-01-16 18:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
archive: use GHashTable for the entries (3.36 KB, patch)
2017-01-15 11:45 UTC, Paolo Borelli
none Details | Review
patch (3.78 KB, patch)
2017-01-15 13:25 UTC, Paolo Borelli
committed Details | Review

Description Paolo Borelli 2017-01-15 11:45:04 UTC
We only use entries as a set to check if a specific entry is present.
Using GHashTable is simpler and avoids walking the list each time.
Comment 1 Paolo Borelli 2017-01-15 11:45:08 UTC
Created attachment 343496 [details] [review]
archive: use GHashTable for the entries
Comment 2 Paolo Borelli 2017-01-15 11:49:59 UTC
Review of attachment 343496 [details] [review]:

Ops, sorry I need to change the hash table to be caseless
Comment 3 Paolo Borelli 2017-01-15 13:25:44 UTC
Created attachment 343499 [details] [review]
patch
Comment 4 Carlos Garcia Campos 2017-01-16 15:42:34 UTC
Review of attachment 343499 [details] [review]:

I agree, thanks!

::: libgxps/gxps-archive.c
@@ +214,3 @@
+                gconstpointer v2)
+{
+    return (g_ascii_strcasecmp(v1, v2) == 0);

I don't think the () are needed here for the return.

@@ +321,3 @@
 {
+	if (path == NULL)
+		return FALSE;

Good catch, I think it actually never happened, but we could be passing NULL to g_ascii_strcasecmp in current code.
Comment 5 Paolo Borelli 2017-01-16 18:32:24 UTC
Comment on attachment 343499 [details] [review]
patch

Amended and pushed, thanks for the quick review!
Comment 6 Paolo Borelli 2017-01-16 18:32:59 UTC
This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.