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 753353 - fix gom_filter_new_sql with gobject introspection
fix gom_filter_new_sql with gobject introspection
Status: RESOLVED FIXED
Product: gom
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Gom Maintainers
Gom Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-08-07 12:40 UTC by Christian Hergert
Modified: 2015-08-12 08:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
copy the values into a new array (2.64 KB, patch)
2015-08-07 12:40 UTC, Christian Hergert
none Details | Review
Add a gom_filter_new_sql unit test (2.25 KB, patch)
2015-08-07 14:52 UTC, Mathieu Bridon
committed Details | Review
copy the values into a new array (2.88 KB, patch)
2015-08-07 17:18 UTC, Christian Hergert
committed Details | Review

Description Christian Hergert 2015-08-07 12:40:09 UTC
Created attachment 308892 [details] [review]
copy the values into a new array

Small fix to copy values when creating from a new GArray<GValue>. I think this is what we need to fix using this function from GObject Introspection.
Comment 1 Mathieu Bridon 2015-08-07 13:03:39 UTC
This indeed fixes the issue I was seeing with Python GI bindings.

Without this patch, the GI stack would unset the GValue, and because it wasn't copied into the filter, then by the time find_async function was called the GValue would be null.

With this patch applied, I can use an SQL filter like so:

    filter = Gom.Filter.new_sql("items.name == ?", ["item2"])
    group = repository.find_async(ItemResource, filter, find_cb, None)

And it just works.
Comment 2 Mathieu Bridon 2015-08-07 13:06:19 UTC
If you copy the GArray into the filter, shouldn't you also free it in gom_filter_finalize?
Comment 3 Mathieu Bridon 2015-08-07 14:52:53 UTC
Created attachment 308904 [details] [review]
Add a gom_filter_new_sql unit test
Comment 4 Mathieu Bridon 2015-08-07 14:55:59 UTC
The unit test I just attached to this bug report also shows the problem that is fixed by Christian's patch.

Without applying it, the unit test fails (because I unset the GValue right after creating the filter), but once I apply the patch, then the unit test just works.

So that allows triggering the problem, without going through GObject-Introspection and Python.
Comment 5 Christian Hergert 2015-08-07 17:18:42 UTC
Created attachment 308908 [details] [review]
copy the values into a new array

Oh look at that, we were previously leaking the GArray too! I figured that was being handled correctly so I didn't even check.

New patch with fix to free the array on finalize.
Comment 6 Bastien Nocera 2015-08-08 11:13:24 UTC
Review of attachment 308904 [details] [review]:

++
Comment 7 Bastien Nocera 2015-08-08 11:20:10 UTC
Review of attachment 308908 [details] [review]:

Looks good as well.
Comment 8 Mathieu Bridon 2015-08-12 08:52:00 UTC
Attachment 308908 [details] pushed as 7d18349 - copy the values into a new array
Attachment 308904 [details] pushed as 0e0ef51 - Add a gom_filter_new_sql unit test