[gegl] operations, meson: make gegl:introspect more robust.



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]