libgimp, plug-ins: remove n_procedures from (init|query)_procedures().

The way currently implemented plug-ins are, they are already
NULL-terminating the returned arrays. Since a procedure name cannot be
NULL itself by definition, defining the array length by a terminal NULL
is enough. There is no need to also add a n_procedures parameters which
is just one more possible bug source, even more as we were already
expecting the NULL termination by using g_strfreev() to free the memory.

Anyway a length parameter does not bring any advantage since a plug-in
can still "lie" about its array size (just as it can forget to
NULL-terminate it) and when this happens, the plug-in will segfault.
That's it, it's just a plug-in programming error.

Last but not least, some binding seem to have issues with returned array
setting an (out) parameter as the length. In pygobject at least, the
length parameter doesn't disappear and we end up with this ugly
signature:

> In [3]: Gimp.PlugIn.do_query_procedures.__doc__
> Out[3]: 'query_procedures(self) -> list, n_procedures:int'

See bug report pygobject#352.
To avoid this, we should either set both the array and the length as
(out) parameters or just set the returned array as NULL-terminated
(which is the solution I chose).
This commit is contained in:
Jehan 2019-08-02 13:36:11 +02:00
parent 3945701bd6
commit bc7b358802
5 changed files with 15 additions and 31 deletions

View File

@ -94,22 +94,19 @@ gimp_plug_in_register (GimpPlugIn *plug_in,
gboolean init)
{
gchar **procedures;
gint n_procedures;
gint i;
gchar **name;
GList *list;
if (init)
procedures = GIMP_PLUG_IN_GET_CLASS (plug_in)->init_procedures (plug_in,
&n_procedures);
procedures = GIMP_PLUG_IN_GET_CLASS (plug_in)->init_procedures (plug_in);
else
procedures = GIMP_PLUG_IN_GET_CLASS (plug_in)->query_procedures (plug_in,
&n_procedures);
procedures = GIMP_PLUG_IN_GET_CLASS (plug_in)->query_procedures (plug_in);
for (i = 0; i < n_procedures; i++)
for (name = procedures; *name; name++)
{
GimpProcedure *procedure;
procedure = gimp_plug_in_create_procedure (plug_in, procedures[i]);
procedure = gimp_plug_in_create_procedure (plug_in, *name);
if (procedure)
{
_gimp_procedure_register (procedure);
@ -118,7 +115,7 @@ gimp_plug_in_register (GimpPlugIn *plug_in,
else
{
g_warning ("Plug-in failed to create procedure '%s'\n",
procedures[i]);
*name);
}
}
g_clear_pointer (&procedures, g_strfreev);

View File

@ -56,7 +56,6 @@ struct _GimpPlugInClass
/**
* GimpPlugInClass::query_procedures:
* @plug_in: a #GimpPlugIn.
* @n_procedures: (out): number of procedures.
*
* This method can be overridden by all plug-ins to return a newly
* allocated array of allocated strings naming the procedures
@ -66,16 +65,14 @@ struct _GimpPlugInClass
*
* See documentation of init_procedures() for differences.
*
* Returns: (array length=n_procedures) (transfer full):
* Returns: (array zero-terminated=1) (transfer full):
* the names of the procedures registered by @plug_in.
*/
gchar ** (* query_procedures) (GimpPlugIn *plug_in,
gint *n_procedures);
gchar ** (* query_procedures) (GimpPlugIn *plug_in);
/**
* GimpPlugInClass::init_procedures:
* @plug_in: a #GimpPlugIn.
* @n_procedures: (out): number of procedures.
*
* This method can be overridden by all plug-ins to return a newly
* allocated array of allocated strings naming procedures registered
@ -92,11 +89,10 @@ struct _GimpPlugInClass
* Most of the time, you only want to override query_procedures() and
* leave init_procedures() untouched.
*
* Returns: (array length=n_procedures) (transfer full):
* Returns: (array zero-terminated=1) (transfer full):
* the names of the procedures registered by @plug_in.
*/
gchar ** (* init_procedures) (GimpPlugIn *plug_in,
gint *n_procedures);
gchar ** (* init_procedures) (GimpPlugIn *plug_in);
/**
* GimpPlugInClass::create_procedure:
* @plug_in: a #GimpPlugIn.

View File

@ -52,8 +52,7 @@ struct _GoatClass
GType goat_get_type (void) G_GNUC_CONST;
static gchar ** goat_query_procedures (GimpPlugIn *plug_in,
gint *n_procedures);
static gchar ** goat_query_procedures (GimpPlugIn *plug_in);
static GimpProcedure * goat_create_procedure (GimpPlugIn *plug_in,
const gchar *name);
@ -82,15 +81,12 @@ goat_init (Goat *goat)
}
static gchar **
goat_query_procedures (GimpPlugIn *plug_in,
gint *n_procedures)
goat_query_procedures (GimpPlugIn *plug_in)
{
gchar **procedures = g_new0 (gchar *, 2);
procedures[0] = g_strdup (PLUG_IN_PROC);
*n_procedures = 1;
return procedures;
}

View File

@ -70,8 +70,7 @@ typedef struct
GType help_get_type (void) G_GNUC_CONST;
static gchar ** help_query_procedures (GimpPlugIn *plug_in,
gint *n_procedures);
static gchar ** help_query_procedures (GimpPlugIn *plug_in);
static GimpProcedure * help_create_procedure (GimpPlugIn *plug_in,
const gchar *name);
@ -114,15 +113,12 @@ help_init (Help *help)
}
static gchar **
help_query_procedures (GimpPlugIn *plug_in,
gint *n_procedures)
help_query_procedures (GimpPlugIn *plug_in)
{
gchar **procedures = g_new0 (gchar *, 2);
procedures[0] = g_strdup (GIMP_HELP_EXT_PROC);
*n_procedures = 1;
return procedures;
}

View File

@ -79,9 +79,8 @@ class PaletteToGradient (Gimp.PlugIn):
self.set_translation_domain ("gimp30-python",
Gio.file_new_for_path(Gimp.locale_directory()))
# XXX See pygobject#352 for the weird return value.
return ['python-fu-palette-to-gradient',
'python-fu-palette-to-gradient-repeating'], 2
'python-fu-palette-to-gradient-repeating']
def do_create_procedure(self, name):
procedure = Gimp.Procedure.new(self, name,