[gegl] operations, meson: make gegl:introspect more robust.
- From: Jehan <jehanp src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gegl] operations, meson: make gegl:introspect more robust.
- Date: Sat, 5 Dec 2020 17:54:16 +0000 (UTC)
commit 5ac40e3c3f9ab25b1208a96a8fdcae6fd72958ef
Author: Jehan <jehan girinstud io>
Date: Sat Dec 5 18:34:44 2020 +0100
operations, meson: make gegl:introspect more robust.
First of all, since "gegl:introspect" is apparently not meant to be an
optional operation, and since it relies on the `dot` program (from
graphviz), we need to search for `dot` in the configuration step (I
could check it exists also on Windows, msys2 even has a package, and it
looks like there are macOS builds and probably BSD too…). If we realize
it is a blocker on some platform, then we should make the whole
operation optional (and unbuilt) rather than leaving it broken.
Also instead of calling `dot` directly, call the program detected at
configuration by passing it through a macro (with an exception for
Windows, where I use "dot.exe" as we will assume everything is on the
same bundled bin/ directory there and build-time paths will be wrong at
runtime).
Thirdly build a temporary filename making sure the file didn't exist
before. On some early tests without the `dot` binary, I even thought it
worked without, but it was only because a file from previous run did
exist in the temporary directory! So let's make proper temp files with
unique names and ensure we delete them after use.
When calling the dot command, do not only check the return value -1
(typically here, I got 127 for a non-existing tool in PATH for
instance), let's consider all non-zero return values as a failure.
Finally when we do consider that `dot` call failed, do not run the GEGL
graph and check that o->user_data is non-NULL before using it. We
therefore avoid various causes of possible crashes.
See also: https://gitlab.gnome.org/GNOME/gimp/-/issues/6045
meson.build | 1 +
operations/common/introspect.c | 92 +++++++++++++++++++++++++++---------------
operations/common/meson.build | 2 +-
3 files changed, 62 insertions(+), 33 deletions(-)
---
diff --git a/meson.build b/meson.build
index 23a9b2400..72581e9fe 100644
--- a/meson.build
+++ b/meson.build
@@ -136,6 +136,7 @@ add_project_arguments(cpp.get_supported_arguments(cflags_cpp), language: 'cpp')
# Utilities
bash = find_program('bash')
+dot = os_win32 ? find_program('dot.exe') : find_program('dot')
perl = find_program('perl5', 'perl', required: false)
asciidoc = find_program('asciidoc', required: false)
diff --git a/operations/common/introspect.c b/operations/common/introspect.c
index e7e34ffd8..a86e12ae7 100644
--- a/operations/common/introspect.c
+++ b/operations/common/introspect.c
@@ -19,6 +19,7 @@
#include "config.h"
#include <stdlib.h>
#include <glib/gi18n-lib.h>
+#include <unistd.h>
#ifdef GEGL_PROPERTIES
@@ -38,49 +39,70 @@ gchar *gegl_to_dot (GeglNode *node);
static void
gegl_introspect_load_cache (GeglProperties *op_introspect)
{
- GeglBuffer *new_buffer = NULL;
- GeglNode *png_load = NULL;
- GeglNode *buffer_sink = NULL;
gchar *dot_string = NULL;
gchar *png_filename = NULL;
gchar *dot_filename = NULL;
gchar *dot_cmd = NULL;
+ gint fd;
if (op_introspect->user_data || op_introspect->node == NULL)
return;
/* Construct temp filenames */
- dot_filename = g_build_filename (g_get_tmp_dir (), "gegl-introspect.dot", NULL);
- png_filename = g_build_filename (g_get_tmp_dir (), "gegl-introspect.png", NULL);
+ dot_filename = g_build_filename (g_get_tmp_dir (), "gegl-introspect-XXXXXX.dot", NULL);
+ png_filename = g_build_filename (g_get_tmp_dir (), "gegl-introspect-XXXXXX.png", NULL);
/* Construct the .dot source */
+ fd = g_mkstemp (dot_filename);
dot_string = gegl_to_dot (GEGL_NODE (op_introspect->node));
- g_file_set_contents (dot_filename, dot_string, -1, NULL);
+ write (fd, dot_string, strlen (dot_string));
+ close (fd);
- /* Process the .dot to a .png */
- dot_cmd = g_strdup_printf ("dot -o %s -Tpng %s", png_filename, dot_filename);
- if (system (dot_cmd) == -1)
- g_warning ("Error executing GraphViz dot program");
-
- /* Create a graph that loads the png into a GeglBuffer and process
- * it
+ /* The only point of using g_mkstemp() here is creating a new file and making
+ * sure we don't override a file which existed before.
+ * Also png_filename will be modified in-place to the actual path name
+ * generated as being unique.
*/
- png_load = gegl_node_new_child (NULL,
- "operation", "gegl:png-load",
- "path", png_filename,
- NULL);
- buffer_sink = gegl_node_new_child (NULL,
- "operation", "gegl:buffer-sink",
- "buffer", &new_buffer,
- NULL);
- gegl_node_link_many (png_load, buffer_sink, NULL);
- gegl_node_process (buffer_sink);
-
- op_introspect->user_data= new_buffer;
+ fd = g_mkstemp (png_filename);
+ close (fd);
+
+ /* Process the .dot to a .png */
+ dot_cmd = g_strdup_printf ("%s -o %s -Tpng %s", DOT, png_filename, dot_filename);
+ if (system (dot_cmd) != 0)
+ {
+ g_warning ("Error executing GraphViz dot program");
+ }
+ else
+ {
+ GeglBuffer *new_buffer = NULL;
+ GeglNode *png_load = NULL;
+ GeglNode *buffer_sink = NULL;
+
+ /* Create a graph that loads the png into a GeglBuffer and process
+ * it
+ */
+ png_load = gegl_node_new_child (NULL,
+ "operation", "gegl:png-load",
+ "path", png_filename,
+ NULL);
+ buffer_sink = gegl_node_new_child (NULL,
+ "operation", "gegl:buffer-sink",
+ "buffer", &new_buffer,
+ NULL);
+ gegl_node_link_many (png_load, buffer_sink, NULL);
+ gegl_node_process (buffer_sink);
+
+ op_introspect->user_data= new_buffer;
+
+ g_object_unref (buffer_sink);
+ g_object_unref (png_load);
+ }
+
+ /* Do not keep the files around. */
+ unlink (dot_filename);
+ unlink (png_filename);
/* Cleanup */
- g_object_unref (buffer_sink);
- g_object_unref (png_load);
g_free (dot_string);
g_free (dot_cmd);
g_free (dot_filename);
@@ -102,15 +124,21 @@ gegl_introspect_get_bounding_box (GeglOperation *operation)
{
GeglRectangle result = {0,0,0,0};
GeglProperties *o = GEGL_PROPERTIES (operation);
- gint width, height;
gegl_introspect_load_cache (o);
- g_object_get (o->user_data, "width", &width,
- "height", &height, NULL);
+ if (o->user_data)
+ {
+ gint width, height;
+
+ g_object_get (o->user_data,
+ "width", &width,
+ "height", &height,
+ NULL);
- result.width = width;
- result.height = height;
+ result.width = width;
+ result.height = height;
+ }
return result;
}
diff --git a/operations/common/meson.build b/operations/common/meson.build
index 13575b571..a7a2b7dd2 100644
--- a/operations/common/meson.build
+++ b/operations/common/meson.build
@@ -140,7 +140,7 @@ gegl_common = shared_library('gegl-common',
gegl_lib,
],
c_args: [
- '-DGEGL_OP_BUNDLE',
+ '-DGEGL_OP_BUNDLE', '-DDOT="' + (os_win32 ? 'dot.exe' : dot.path()) + '"'
],
name_prefix: '',
install: true,
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]