GNOME Bugzilla – Bug 766636
Add grl.resource()
Last modified: 2018-09-24 09:43:42 UTC
.
Created attachment 328158 [details] [review] lua-factory: Fix grl.fetch API doc
Created attachment 328159 [details] [review] lua-factory: Fix GResource leak When registering and deregistering a source, we should make sure that its resources get unregistered as well.
Created attachment 328160 [details] [review] lua-factory: Make _load_goa_data() a private function It's only used inside the grl-lua-library.c source.
Created attachment 328161 [details] [review] lua-factory: Add grl.resource() To allow sources to load files inside a private resources bundle. So, for a grl-foo.lua source to load the "public" GResources, which will get registered so that the application can access it, you'd name the resource file grl-foo.gresource And to load the private GResources data that only the source itself can access, you'd name the resource file grl-foo-private.gresource
Review of attachment 328158 [details] [review]: ok
Review of attachment 328159 [details] [review]: great
Review of attachment 328160 [details] [review]: sure
Review of attachment 328161 [details] [review]: Looks good. A unit test for this would be great :) I'll mark as reviewed so you can double check if leaks on failure. ::: src/lua-factory/grl-lua-factory.c @@ +452,3 @@ resource = NULL; + source->priv->private_resource = priv_resource; + priv_resource = NULL; I don't see where both resources are freed if we `goto bail` below.. we probably want to g_clear_object (&source) there.
Is it possible to return nil if the resource doesn't exist or fails to load? Trying to load a non existing resource crashes.
Is there any way to configure a plugin with resource URIs from the application so the plugin can use these as sources? Currently, using resources from the application doesn't work.
(In reply to Adrien Plazas from comment #9) > Is it possible to return nil if the resource doesn't exist or fails to load? > Trying to load a non existing resource crashes. It's supposed to return nil already. Backtrace and test case please. (In reply to Adrien Plazas from comment #10) > Is there any way to configure a plugin with resource URIs from the > application so the plugin can use these as sources? > > Currently, using resources from the application doesn't work. No. Otherwise that means that the source can access any GResource data from other sources.
Attachment 328158 [details] pushed as 4fdd9fd - lua-factory: Fix grl.fetch API doc Attachment 328159 [details] pushed as fb8a8d6 - lua-factory: Fix GResource leak Attachment 328160 [details] pushed as fd67192 - lua-factory: Make _load_goa_data() a private function
(In reply to Victor Toso from comment #8) > ::: src/lua-factory/grl-lua-factory.c > @@ +452,3 @@ > resource = NULL; > + source->priv->private_resource = priv_resource; > + priv_resource = NULL; > > I don't see where both resources are freed if we `goto bail` below.. we > probably want to g_clear_object (&source) there. Besides this, I don't see a reason to hold this patch.
(In reply to Victor Toso from comment #13) > (In reply to Victor Toso from comment #8) > > ::: src/lua-factory/grl-lua-factory.c > > @@ +452,3 @@ > > resource = NULL; > > + source->priv->private_resource = priv_resource; > > + priv_resource = NULL; > > > > I don't see where both resources are freed if we `goto bail` below.. we > > probably want to g_clear_object (&source) there. > > Besides this, I don't see a reason to hold this patch. We're waiting for the crash report from Adrien :/
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/grilo/issues/88.