From 7189010f1e1fd42d7b02d00e05472cd559d5aa9f Mon Sep 17 00:00:00 2001 From: Jehan Date: Thu, 3 Jul 2025 00:16:09 +0200 Subject: [PATCH] app, pdb, libimp: prevent core process from raising WARNING on libgimp call. Whatever a plug-in does, it should not be able to trigger WARNINGs or CRITICALs on the core process. Here this was possible when requesting an array of param specs with gimp_drawable_filter_operation_get_pspecs() on a GEGL operation, while GIMP doesn't support all types for this filter's arguments. Trying to send unsupported type specs through the wire would fail. Unfortunately just saying that we must add support for these types is not enough because we simply cannot support every possible types. First because even in current GEGL core filters, there are types we might never support (e.g. audio fragments?). But even more because third-party filters could have custom types too (just like our own filters have custom GIMP types). So instead, acknowledge that some types cannot be sent through the wire, verify when it's one such argument and simply output an informational message on stderr (because the info of a non-supported type is still interesting in case this is something we should in fact add support for; it's much better than silently ignoring the argument). --- app/pdb/drawable-filter-cmds.c | 41 +++++++++++++++++++++++++++----- app/plug-in/plug-in-rc.c | 2 +- libgimp/gimpgpparams-body.c | 22 ++++++++++++----- libgimp/gimpgpparams.h | 5 ++-- libgimp/gimpprocedure.c | 4 ++-- pdb/app.pl | 5 +++- pdb/groups/drawable_filter.pdb | 43 +++++++++++++++++++++++++++++----- 7 files changed, 98 insertions(+), 24 deletions(-) diff --git a/app/pdb/drawable-filter-cmds.c b/app/pdb/drawable-filter-cmds.c index 1823750b14..b4ad832472 100644 --- a/app/pdb/drawable-filter-cmds.c +++ b/app/pdb/drawable-filter-cmds.c @@ -28,10 +28,15 @@ #include +#include "libgimpbase/gimpbase.h" +#include "libgimpbase/gimpprotocol.h" +#include "libgimpbase/gimpwire.h" + #include "libgimpbase/gimpbase.h" #include "pdb-types.h" +#include "libgimp/gimpgpparams.h" #include "core/gimpcontainer.h" #include "core/gimpdrawable-filters.h" #include "core/gimpdrawable.h" @@ -855,15 +860,39 @@ drawable_filter_operation_get_pspecs_invoker (GimpProcedure *procedure, for (gint i = 0; i < n_specs; i++) { - GParamSpec *pspec = specs[n_parent_specs + i]; - GValue value = G_VALUE_INIT; + GParamSpec *pspec = specs[n_parent_specs + i]; + GPParamDef param_def = { 0, }; - g_value_init (&value, G_TYPE_PARAM); + /* Make sure we do not try to send param specs over the wire + * if we don't support sending their type. + */ + if (_gimp_param_spec_to_gp_param_def (pspec, ¶m_def, TRUE)) + { + GValue value = G_VALUE_INIT; - g_value_set_param (&value, g_param_spec_ref (pspec)); - gimp_value_array_append (pspecs, &value); + g_value_init (&value, G_TYPE_PARAM); - g_value_unset (&value); + g_value_set_param (&value, g_param_spec_ref (pspec)); + gimp_value_array_append (pspecs, &value); + + g_value_unset (&value); + } + else + { + /* This is not technically a bug if an operation has + * unsupported argument types, because we cannot possibly + * support everything (third-party operations could have any + * kind of arguments; and even official ops have types + * such as audio fragment which we may never support). + * So this should not generate any WARNING. Yet we still + * want to softly notify developers, in case we can + * actually do something about some types. + */ + g_printerr ("%s: ignoring argument '%s' of procedure '%s'. " + "Unsupported argument type '%s'.\n", + G_STRFUNC, pspec->name, operation_name, + g_type_name (G_PARAM_SPEC_VALUE_TYPE (pspec))); + } } g_free (specs); diff --git a/app/plug-in/plug-in-rc.c b/app/plug-in/plug-in-rc.c index ddf632fc28..314b7e3252 100644 --- a/app/plug-in/plug-in-rc.c +++ b/app/plug-in/plug-in-rc.c @@ -1152,7 +1152,7 @@ plug_in_rc_write_proc_arg (GimpConfigWriter *writer, { GPParamDef param_def = { 0, }; - _gimp_param_spec_to_gp_param_def (pspec, ¶m_def); + _gimp_param_spec_to_gp_param_def (pspec, ¶m_def, FALSE); gimp_config_writer_open (writer, "proc-arg"); gimp_config_writer_printf (writer, "%d", param_def.param_def_type); diff --git a/libgimp/gimpgpparams-body.c b/libgimp/gimpgpparams-body.c index 5a5f619a7b..afed740c41 100644 --- a/libgimp/gimpgpparams-body.c +++ b/libgimp/gimpgpparams-body.c @@ -367,12 +367,14 @@ _gimp_gp_param_def_to_param_spec (const GPParamDef *param_def) return NULL; } -void +gboolean _gimp_param_spec_to_gp_param_def (GParamSpec *pspec, - GPParamDef *param_def) + GPParamDef *param_def, + gboolean check_only) { GType pspec_type = G_PARAM_SPEC_TYPE (pspec); GType value_type = G_PARAM_SPEC_VALUE_TYPE (pspec); + gboolean success = TRUE; param_def->param_def_type = GP_PARAM_DEF_TYPE_DEFAULT; param_def->type_name = (gchar *) g_type_name (pspec_type); @@ -767,11 +769,19 @@ _gimp_param_spec_to_gp_param_def (GParamSpec *pspec, } else { - g_warning ("%s: GParamSpecObject for unsupported type '%s:%s'", - G_STRFUNC, - param_def->type_name, param_def->value_type_name); + success = FALSE; + if (! check_only) + g_warning ("%s: GParamSpecObject for unsupported type '%s:%s'", + G_STRFUNC, + param_def->type_name, param_def->value_type_name); } } + else + { + success = FALSE; + } + + return success; } static GimpImage * @@ -1709,7 +1719,7 @@ gimp_value_to_gp_param (const GValue *value, param->param_type = GP_PARAM_TYPE_PARAM_DEF; _gimp_param_spec_to_gp_param_def (g_value_get_param (value), - ¶m->data.d_param_def); + ¶m->data.d_param_def, FALSE); } else if (GIMP_VALUE_HOLDS_VALUE_ARRAY (value)) { diff --git a/libgimp/gimpgpparams.h b/libgimp/gimpgpparams.h index 8f88080068..1edd91d131 100644 --- a/libgimp/gimpgpparams.h +++ b/libgimp/gimpgpparams.h @@ -26,8 +26,9 @@ G_BEGIN_DECLS GParamSpec * _gimp_gp_param_def_to_param_spec (const GPParamDef *param_def); -void _gimp_param_spec_to_gp_param_def (GParamSpec *pspec, - GPParamDef *param_def); +gboolean _gimp_param_spec_to_gp_param_def (GParamSpec *pspec, + GPParamDef *param_def, + gboolean check_only); GimpValueArray * _gimp_gp_params_to_value_array (gpointer gimp, GParamSpec **pspecs, diff --git a/libgimp/gimpprocedure.c b/libgimp/gimpprocedure.c index db7bb88432..60dbe2430a 100644 --- a/libgimp/gimpprocedure.c +++ b/libgimp/gimpprocedure.c @@ -411,13 +411,13 @@ gimp_procedure_real_install (GimpProcedure *procedure) for (i = 0; i < n_args; i++) { _gimp_param_spec_to_gp_param_def (args[i], - &proc_install.params[i]); + &proc_install.params[i], FALSE); } for (i = 0; i < n_return_vals; i++) { _gimp_param_spec_to_gp_param_def (return_vals[i], - &proc_install.return_vals[i]); + &proc_install.return_vals[i], FALSE); } plug_in = gimp_procedure_get_plug_in (procedure); diff --git a/pdb/app.pl b/pdb/app.pl index 3294a06ba3..34f7ad1bd2 100644 --- a/pdb/app.pl +++ b/pdb/app.pl @@ -1111,6 +1111,9 @@ GPL elsif (!/libgimp/) { s/^/~/; } + elsif (/libgimp[a-z]/) { + s/^/"/; + } } $x cmp $y; } keys %{$out->{headers}}; @@ -1137,7 +1140,7 @@ GPL $seen = 0 if !/^name, operation_name, + g_type_name (G_PARAM_SPEC_VALUE_TYPE (pspec))); + } } g_free (specs); @@ -903,6 +927,13 @@ CODE @headers = qw( + + "libgimpbase/gimpbase.h" + "libgimpbase/gimpprotocol.h" + "libgimpbase/gimpwire.h" + + "libgimp/gimpgpparams.h" + "core/gimpcontainer.h" "core/gimpdrawablefilter.h" "core/gimpdrawable-filters.h"