gplugin/gplugin

Make the basic plugins verify that they've been called with the correct parameters

2021-07-23, Gary Kramlich
84288a17a5b8
Parents 8587dea596f6
Children bf9af95cb6d4
Make the basic plugins verify that they've been called with the correct parameters

Testing Done:
Ran the unit tests but the perl load is being a pain and I'm considering dropping it. See the comment in the code for more information.

Reviewed at https://reviews.imfreedom.org/r/817/
11 files changed, 206 insertions(+), 94 deletions(-)
  • +16 -2
    gplugin/tests/plugins/basic-plugin.c
  • +8 -0
    lua/tests/plugins/basic.lua
  • +114 -73
    perl5/gplugin-perl5-loader.c
  • +15 -1
    perl5/tests/plugins/basic.pl
  • +5 -4
    perl5/tests/plugins/dependent.pl
  • +5 -4
    perl5/tests/plugins/load-exception.pl
  • +12 -6
    python3/gplugin-python3-loader.c
  • +4 -2
    python3/gplugin-python3-utils.c
  • +7 -0
    python3/tests/plugins/basic.py
  • +9 -1
    vala/tests/genie-plugins/basic.gs
  • +11 -1
    vala/tests/plugins/basic.vala
    • --- a/gplugin/tests/plugins/basic-plugin.c Thu Jul 15 00:44:27 2021 -0500
    +++ b/gplugin/tests/plugins/basic-plugin.c Fri Jul 23 04:07:52 2021 -0500
    @@ -40,14 +40,28 @@
    }
    static gboolean
    -basic_load(G_GNUC_UNUSED GPluginPlugin *plugin, G_GNUC_UNUSED GError **error)
    +basic_load(GPluginPlugin *plugin, GError **error)
    {
    + if(!GPLUGIN_IS_PLUGIN(plugin)) {
    + g_set_error_literal(error, GPLUGIN_DOMAIN, 0, "plugin was not set");
    +
    + return FALSE;
    + }
    +
    return TRUE;
    }
    static gboolean
    -basic_unload(G_GNUC_UNUSED GPluginPlugin *plugin, G_GNUC_UNUSED GError **error)
    +basic_unload(
    + GPluginPlugin *plugin,
    + GError **error)
    {
    + if(!GPLUGIN_IS_PLUGIN(plugin)) {
    + g_set_error_literal(error, GPLUGIN_DOMAIN, 0, "plugin was not set");
    +
    + return FALSE;
    + }
    +
    return TRUE;
    }
    --- a/lua/tests/plugins/basic.lua Thu Jul 15 00:44:27 2021 -0500
    +++ b/lua/tests/plugins/basic.lua Fri Jul 23 04:07:52 2021 -0500
    @@ -34,9 +34,17 @@
    end
    function gplugin_load(plugin)
    + if plugin == nil then
    + error('plugin is nil')
    + end
    +
    return true
    end
    function gplugin_unload(plugin)
    + if plugin == nil then
    + error('plugin is nil')
    + end
    +
    return true
    end
    --- a/perl5/gplugin-perl5-loader.c Thu Jul 15 00:44:27 2021 -0500
    +++ b/perl5/gplugin-perl5-loader.c Fri Jul 23 04:07:52 2021 -0500
    @@ -77,6 +77,7 @@
    {
    GPluginPluginInfo *info = NULL;
    PerlInterpreter *old = NULL;
    + SV *err_tmp;
    gint ret = 0;
    dSP;
    @@ -95,17 +96,19 @@
    SPAGAIN;
    - if(ret != 1) {
    - g_set_error_literal(
    - error,
    - GPLUGIN_DOMAIN,
    - 0,
    - "gplugin_query did not return a GPluginPluginInfo");
    + /* ERRSV is a macro, so we store it instead of calling it multiple times. */
    + err_tmp = ERRSV;
    + if(SvTRUE(err_tmp)) {
    + const gchar *errmsg = SvPVutf8_nolen(err_tmp);
    +
    + g_set_error_literal(error, GPLUGIN_DOMAIN, 0, errmsg);
    } else {
    - if(SvTRUE(ERRSV)) {
    - const gchar *errmsg = SvPVutf8_nolen(ERRSV);
    -
    - g_set_error_literal(error, GPLUGIN_DOMAIN, 0, errmsg);
    + if(ret != 1) {
    + g_set_error_literal(
    + error,
    + GPLUGIN_DOMAIN,
    + 0,
    + "gplugin_query did not return a GPluginPluginInfo");
    } else {
    info = (GPluginPluginInfo *)gperl_get_object(POPs);
    @@ -128,59 +131,6 @@
    return info;
    }
    -static gboolean
    -gplugin_perl_loader_call_boolean(
    - PerlInterpreter *interpreter,
    - const gchar *func,
    - GError **error)
    -{
    - PerlInterpreter *old = NULL;
    - gboolean r = FALSE;
    - gint count = 0;
    -
    - dSP;
    -
    - old = my_perl;
    - my_perl = interpreter;
    - PERL_SET_CONTEXT(interpreter);
    -
    - ENTER;
    - SAVETMPS;
    - PUSHMARK(SP);
    - PUTBACK;
    -
    - count = call_pv(func, G_EVAL | G_SCALAR);
    -
    - SPAGAIN;
    -
    - if(count != 1) {
    - g_set_error(
    - error,
    - GPLUGIN_DOMAIN,
    - 0,
    - "%s did not return a value",
    - func);
    - } else {
    - if(SvTRUE(ERRSV)) {
    - const gchar *errmsg = SvPVutf8_nolen(ERRSV);
    -
    - g_set_error_literal(error, GPLUGIN_DOMAIN, 0, errmsg);
    - } else {
    - if(POPi == 0) {
    - r = TRUE;
    - }
    - }
    - }
    -
    - PUTBACK;
    - FREETMPS;
    - LEAVE;
    -
    - my_perl = old;
    -
    - return r;
    -}
    -
    /******************************************************************************
    * GPluginLoaderInterface API
    *****************************************************************************/
    @@ -243,7 +193,9 @@
    info = gplugin_perl_loader_call_gplugin_query(interpreter, error);
    if(!GPLUGIN_IS_PLUGIN_INFO(info)) {
    - g_set_error_literal(error, GPLUGIN_DOMAIN, 0, "failed to query");
    + if(error != NULL && *error == NULL) {
    + g_set_error_literal(error, GPLUGIN_DOMAIN, 0, "failed to query");
    + }
    return NULL;
    }
    @@ -268,11 +220,60 @@
    GError **error)
    {
    GPluginPerlPlugin *pplugin = GPLUGIN_PERL_PLUGIN(plugin);
    - PerlInterpreter *interpreter = NULL;
    + PerlInterpreter *old = NULL;
    + SV *err_tmp = NULL;
    + gboolean r = FALSE;
    + gint count = 0;
    +
    + dSP;
    +
    + old = my_perl;
    + my_perl = gplugin_perl_plugin_get_interpreter(pplugin);
    + PERL_SET_CONTEXT(my_perl);
    +
    + ENTER;
    + SAVETMPS;
    + PUSHMARK(SP);
    + EXTEND(SP, 1);
    + PUSHs(sv_2mortal(newSVGObject(G_OBJECT(pplugin))));
    +
    + PUTBACK;
    + count = call_pv("gplugin_load", G_EVAL | G_SCALAR);
    + SPAGAIN;
    +
    + /* ERRSV is a macro, so we store it instead of calling it multiple times. */
    + err_tmp = ERRSV;
    + if(SvTRUE(err_tmp)) {
    + const gchar *errmsg = SvPVutf8_nolen(err_tmp);
    - interpreter = gplugin_perl_plugin_get_interpreter(pplugin);
    + g_set_error_literal(error, GPLUGIN_DOMAIN, 0, errmsg);
    + } else {
    + if(count != 1) {
    + g_set_error_literal(
    + error,
    + GPLUGIN_DOMAIN,
    + 0,
    + "gplugin_load did not return a value");
    + } else {
    + if(POPi == 0) {
    + r = TRUE;
    + }
    + }
    + }
    - return gplugin_perl_loader_call_boolean(interpreter, "gplugin_load", error);
    + PUTBACK;
    + FREETMPS;
    + LEAVE;
    +
    + my_perl = old;
    +
    + /* this is magic and is keeping this working. Why I don't know, but we're
    + * debating chucking this loader out the window and I want to create this
    + * review request so I'm leaving it in for now...
    + */
    + g_message("load returning: %d for %s", r, gplugin_plugin_get_filename(plugin));
    +
    + return r;
    }
    static gboolean
    @@ -282,14 +283,54 @@
    GError **error)
    {
    GPluginPerlPlugin *pplugin = GPLUGIN_PERL_PLUGIN(plugin);
    - PerlInterpreter *interpreter = NULL;
    + PerlInterpreter *old = NULL;
    + SV *err_tmp = NULL;
    + gboolean r = FALSE;
    + gint count = 0;
    +
    + dSP;
    - interpreter = gplugin_perl_plugin_get_interpreter(pplugin);
    + old = my_perl;
    + my_perl = gplugin_perl_plugin_get_interpreter(pplugin);
    + PERL_SET_CONTEXT(my_perl);
    +
    + ENTER;
    + SAVETMPS;
    + PUSHMARK(SP);
    + EXTEND(SP, 1);
    + PUSHs(sv_2mortal(newSVGObject(G_OBJECT(pplugin))));
    +
    + PUTBACK;
    + count = call_pv("gplugin_unload", G_EVAL | G_SCALAR);
    + SPAGAIN;
    - return gplugin_perl_loader_call_boolean(
    - interpreter,
    - "gplugin_unload",
    - error);
    + /* ERRSV is a macro, so we store it instead of calling it multiple times. */
    + err_tmp = ERRSV;
    + if(SvTRUE(err_tmp)) {
    + const gchar *errmsg = SvPVutf8_nolen(err_tmp);
    +
    + g_set_error_literal(error, GPLUGIN_DOMAIN, 0, errmsg);
    + } else {
    + if(count != 1) {
    + g_set_error_literal(
    + error,
    + GPLUGIN_DOMAIN,
    + 0,
    + "gplugin_unload did not return a value");
    + } else {
    + if(POPi == 0) {
    + r = TRUE;
    + }
    + }
    + }
    +
    + PUTBACK;
    + FREETMPS;
    + LEAVE;
    +
    + my_perl = old;
    +
    + return r;
    }
    /******************************************************************************
    --- a/perl5/tests/plugins/basic.pl Thu Jul 15 00:44:27 2021 -0500
    +++ b/perl5/tests/plugins/basic.pl Fri Jul 23 04:07:52 2021 -0500
    @@ -16,8 +16,10 @@
    use strict;
    use Glib::Object::Introspection;
    +use Scalar::Util qw(blessed);
    -Glib::Object::Introspection->setup(basename => "GPlugin", version => "1.0", package=> "GPlugin");
    +Glib::Object::Introspection->setup(basename => "GPlugin", version => "1.0",
    + package => "GPlugin");
    sub gplugin_query {
    return GPlugin::PluginInfo->new(
    @@ -35,9 +37,21 @@
    }
    sub gplugin_load {
    + my $plugin = shift;
    +
    + if(!defined($plugin) or (blessed($plugin) and !$plugin->isa("GPlugin::Plugin"))) {
    + die("plugin is not a GPlugin::Plugin");
    + }
    +
    return 0;
    }
    sub gplugin_unload {
    + my $plugin = shift;
    +
    + if(!defined($plugin) or (blessed($plugin) and !$plugin->isa("GPlugin::Plugin"))) {
    + die("plugin is not a GPlugin::Plugin");
    + }
    +
    return 0;
    }
    --- a/perl5/tests/plugins/dependent.pl Thu Jul 15 00:44:27 2021 -0500
    +++ b/perl5/tests/plugins/dependent.pl Fri Jul 23 04:07:52 2021 -0500
    @@ -17,19 +17,20 @@
    use Glib::Object::Introspection;
    -Glib::Object::Introspection->setup(basename => "GPlugin", version => "1.0", package=> "GPlugin");
    +Glib::Object::Introspection->setup(basename => "GPlugin", version => "1.0",
    + package => "GPlugin");
    -sub gplugin_query() {
    +sub gplugin_query {
    return GPlugin::PluginInfo->new(
    id => "gplugin/perl5-dependent-plugin",
    dependencies => ['dependency1', 'dependency2'],
    );
    }
    -sub gplugin_load() {
    +sub gplugin_load {
    return 1;
    }
    -sub gplugin_unload() {
    +sub gplugin_unload {
    return 1;
    }
    --- a/perl5/tests/plugins/load-exception.pl Thu Jul 15 00:44:27 2021 -0500
    +++ b/perl5/tests/plugins/load-exception.pl Fri Jul 23 04:07:52 2021 -0500
    @@ -17,20 +17,21 @@
    use Glib::Object::Introspection;
    -Glib::Object::Introspection->setup(basename => "GPlugin", version => "1.0", package=> "GPlugin");
    +Glib::Object::Introspection->setup(basename => "GPlugin", version => "1.0",
    + package => "GPlugin");
    -sub gplugin_query() {
    +sub gplugin_query {
    return GPlugin::PluginInfo->new(
    id => "gplugin/perl5-load-exception",
    );
    }
    -sub gplugin_load() {
    +sub gplugin_load {
    die("you be dead");
    return 0;
    }
    -sub gplugin_unload() {
    +sub gplugin_unload {
    return 0;
    }
    --- a/python3/gplugin-python3-loader.c Thu Jul 15 00:44:27 2021 -0500
    +++ b/python3/gplugin-python3-loader.c Fri Jul 23 04:07:52 2021 -0500
    @@ -1,5 +1,5 @@
    /*
    - * Copyright (C) 2011-2020 Gary Kramlich <grim@reaperworld.com>
    + * Copyright (C) 2011-2021 Gary Kramlich <grim@reaperworld.com>
    *
    * This library is free software; you can redistribute it and/or
    * modify it under the terms of the GNU Lesser General Public
    @@ -80,7 +80,10 @@
    module = PyImport_ImportModuleEx(module_name, NULL, NULL, package_list);
    if(PyErr_Occurred()) {
    g_warning(_("Failed to query %s"), filename);
    - PyErr_Print();
    +
    + if(error != NULL) {
    + *error = gplugin_python3_exception_to_gerror();
    + }
    /* clean some stuff up */
    g_free(module_name);
    @@ -219,7 +222,7 @@
    if(PyErr_Occurred()) {
    Py_XDECREF(result);
    - if(error) {
    + if(error != NULL) {
    *error = gplugin_python3_exception_to_gerror();
    }
    @@ -246,7 +249,8 @@
    GPluginPlugin *plugin,
    GError **error)
    {
    - PyObject *unload = NULL, *pyplugin = NULL, *result = NULL;
    + PyObject *unload = NULL, *result = NULL;
    + PyObject *pyplugin = NULL;
    gboolean ret = FALSE;
    g_object_get(G_OBJECT(plugin), "unload-func", &unload, NULL);
    @@ -257,9 +261,11 @@
    Py_DECREF(pyplugin);
    if(PyErr_Occurred()) {
    - PyErr_Print();
    + Py_XDECREF(result);
    - Py_XDECREF(result);
    + if(error != NULL) {
    + *error = gplugin_python3_exception_to_gerror();
    + }
    return FALSE;
    }
    --- a/python3/gplugin-python3-utils.c Thu Jul 15 00:44:27 2021 -0500
    +++ b/python3/gplugin-python3-utils.c Fri Jul 23 04:07:52 2021 -0500
    @@ -78,12 +78,14 @@
    PyObject *type = NULL, *value = NULL, *trace = NULL;
    PyObject *type_name = NULL, *value_str = NULL, *obj = NULL;
    - if(!PyErr_Occurred())
    + if(!PyErr_Occurred()) {
    return NULL;
    + }
    PyErr_Fetch(&type, &value, &trace);
    - if(type == NULL)
    + if(type == NULL) {
    return NULL;
    + }
    PyErr_NormalizeException(&type, &value, &trace);
    Py_XDECREF(trace);
    --- a/python3/tests/plugins/basic.py Thu Jul 15 00:44:27 2021 -0500
    +++ b/python3/tests/plugins/basic.py Fri Jul 23 04:07:52 2021 -0500
    @@ -36,8 +36,15 @@
    def gplugin_load(plugin):
    + if not isinstance(plugin, GPlugin.Plugin):
    + raise Exception('plugin is not a GPlugin.Plugin')
    +
    return True
    def gplugin_unload(plugin):
    + if not isinstance(plugin, GPlugin.Plugin):
    + raise Exception('plugin is not a GPlugin.Plugin')
    +
    return True
    +
    --- a/vala/tests/genie-plugins/basic.gs Thu Jul 15 00:44:27 2021 -0500
    +++ b/vala/tests/genie-plugins/basic.gs Fri Jul 23 04:07:52 2021 -0500
    @@ -39,11 +39,19 @@
    return new BasicPluginInfo()
    def gplugin_load(plugin : GPlugin.Plugin, out error : Error) : bool
    + if not(plugin isa GPlugin.Plugin)
    + error = new Error(Quark.from_string("gplugin"), 0, "plugin was not set")
    + return false
    +
    error = null
    return true
    -def gplugin_unload(plugin : GPlugin.Plugin, out error : Error) : bool
    +def gplugin_unload(plugin : GPlugin.Plugin, shutdown : bool, out error : Error) : bool
    + if not(plugin isa GPlugin.Plugin)
    + error = new Error(Quark.from_string("gplugin"), 0, "plugin was not set")
    + return false
    +
    error = null
    return true
    --- a/vala/tests/plugins/basic.vala Thu Jul 15 00:44:27 2021 -0500
    +++ b/vala/tests/plugins/basic.vala Fri Jul 23 04:07:52 2021 -0500
    @@ -42,12 +42,22 @@
    }
    public bool gplugin_load(GPlugin.Plugin plugin, out Error error) {
    + if(!(plugin is GPlugin.Plugin)) {
    + error = new Error(Quark.from_string("gplugin"), 0, "plugin as not set");
    + return false;
    + }
    +
    error = null;
    return true;
    }
    -public bool gplugin_unload(GPlugin.Plugin plugin, out Error error) {
    +public bool gplugin_unload(GPlugin.Plugin plugin, bool shutdown, out Error error) {
    + if(!(plugin is GPlugin.Plugin)) {
    + error = new Error(Quark.from_string("gplugin"), 0, "plugin as not set");
    + return false;
    + }
    +
    error = null;
    return true;