GNOME.org
 

[gegl/wip/Jehan/gegl_node_process_success: 106/110] gegl, operations, tests: allow running gegl_operation_set_error() in...

commit a60f71e9848fb1b5144284297757c8fc3948b006
Author: Jehan <jehan girinstud io>
Date:   Mon Mar 25 20:17:04 2019 +0100

    gegl, operations, tests: allow running gegl_operation_set_error() in...
    
    ... get_bounding_box() virtual method implementations.
    Typically in gegl:png-load for instance, some fatal errors may occur
    already while calling get_bounding_box(), such as unreadable files, or
    obviously invalid files from early header check.
    Previously when this happened, the GeglOperation processing was going
    forward, raising various CRITICALs. We should detect and stop earlier
    when this happens.
    Also adding tests in the new test-errors unit test, with such a fail
    gegl:png-load operation. Also testing that the error is returned with
    gegl_node_blit_buffer() as well as gegl_node_process().

 gegl/graph/gegl-node.c                    |  13 +++-
 gegl/operation/gegl-operation.c           |   4 +-
 gegl/operation/gegl-operation.h           |   6 +-
 gegl/process/gegl-eval-manager.c          |  22 +++---
 gegl/process/gegl-graph-traversal-debug.c |   4 +-
 gegl/process/gegl-graph-traversal.c       |  15 +++-
 gegl/process/gegl-graph-traversal.h       |   3 +-
 gegl/process/gegl-processor.c             |   8 +++
 operations/external/png-load.c            | 110 +++++++++++++++++------------
 tests/simple/test-errors.c                | 111 +++++++++++++++++++++++++++++-
 10 files changed, 235 insertions(+), 61 deletions(-)
---
diff --git a/gegl/graph/gegl-node.c b/gegl/graph/gegl-node.c
index 9a962fab3..388378ba9 100644
--- a/gegl/graph/gegl-node.c
+++ b/gegl/graph/gegl-node.c
@@ -1847,9 +1847,18 @@ gegl_node_process (GeglNode *self) /* XXX: add level argument?  */
 
   g_return_if_fail (GEGL_IS_NODE (self));
 
+  g_clear_error (&self->error);
   processor = gegl_node_new_processor (self, NULL);
 
-  while (gegl_processor_work (processor, NULL)) ;
+  if (self->error)
+    /* Do not process the graph if we have an error already (this may be an
+     * error while running get_bounding_box() virtual method of an operation
+     * in particular).
+     */
+    self->success = FALSE;
+  else
+    while (gegl_processor_work (processor, NULL)) ;
+
   g_object_unref (processor);
 }
 
@@ -1865,6 +1874,8 @@ gegl_node_process_success (GeglNode  *self,
   else
     real_node = gegl_node_get_output_proxy (self, "output");
 
+  g_return_val_if_fail (real_node, FALSE);
+
   if (error && ! real_node->success && real_node->error)
     *error = g_error_copy (real_node->error);
 
diff --git a/gegl/operation/gegl-operation.c b/gegl/operation/gegl-operation.c
index 4650be2f6..3e8e088ad 100644
--- a/gegl/operation/gegl-operation.c
+++ b/gegl/operation/gegl-operation.c
@@ -202,7 +202,9 @@ gegl_operation_set_error (GeglOperation *operation,
                           GError        *error)
 {
   g_clear_error (&operation->node->error);
-  g_propagate_error (&operation->node->error, error);
+  if (error)
+    g_propagate_prefixed_error (&operation->node->error, error,
+                                "[%s] ", gegl_operation_get_name (operation));
 }
 
 /* Calls an extending class' get_bound_box method if defined otherwise
diff --git a/gegl/operation/gegl-operation.h b/gegl/operation/gegl-operation.h
index 6107d5016..79292b035 100644
--- a/gegl/operation/gegl-operation.h
+++ b/gegl/operation/gegl-operation.h
@@ -204,8 +204,10 @@ gboolean        gegl_operation_process       (GeglOperation *operation,
  *
  * Document the error encountered by an operation.
  * This should typically be used in GeglOperation subclasses when they return
- * #FALSE in their implementation of process(). You should not set an error
- * while returning TRUE.
+ * #FALSE in their implementation of process() virtual method. You should not
+ * set an error while returning TRUE.
+ * You can also call gegl_operation_set_error() within get_bounding_box()
+ * virtual method if a fatal error occured at this early point.
  * Note that @error is propagated to the caller and therefore no longer valid
  * after this call. In particular you should not try to free it (if you wish to
  * reuse the variable, just set it to NULL).
diff --git a/gegl/process/gegl-eval-manager.c b/gegl/process/gegl-eval-manager.c
index 351b9b26e..32216fcd2 100644
--- a/gegl/process/gegl-eval-manager.c
+++ b/gegl/process/gegl-eval-manager.c
@@ -90,7 +90,7 @@ gegl_eval_manager_prepare (GeglEvalManager *self)
       else
         gegl_graph_rebuild (self->traversal, self->node);
 
-      gegl_graph_prepare (self->traversal);
+      gegl_graph_prepare (self->traversal, &self->node->error);
 
       self->state = READY;
     }
@@ -108,7 +108,7 @@ gegl_eval_manager_apply (GeglEvalManager     *self,
                          const GeglRectangle *roi,
                          gint                 level)
 {
-  GeglBuffer  *object;
+  GeglBuffer  *object = NULL;
 
   g_return_val_if_fail (GEGL_IS_EVAL_MANAGER (self), NULL);
   g_return_val_if_fail (GEGL_IS_NODE (self->node), NULL);
@@ -122,13 +122,19 @@ gegl_eval_manager_apply (GeglEvalManager     *self,
   gegl_eval_manager_prepare (self);
   GEGL_INSTRUMENT_END ("gegl", "prepare-graph");
 
-  GEGL_INSTRUMENT_START();
-  gegl_graph_prepare_request (self->traversal, roi, level);
-  GEGL_INSTRUMENT_END ("gegl", "prepare-request");
+  if (! self->node->error)
+    {
+      GEGL_INSTRUMENT_START();
+      gegl_graph_prepare_request (self->traversal, roi, level);
+      GEGL_INSTRUMENT_END ("gegl", "prepare-request");
+    }
 
-  GEGL_INSTRUMENT_START();
-  object = gegl_graph_process (self->traversal, level, &self->node->error);
-  GEGL_INSTRUMENT_END ("gegl", "process");
+  if (! self->node->error)
+    {
+      GEGL_INSTRUMENT_START();
+      object = gegl_graph_process (self->traversal, level, &self->node->error);
+      GEGL_INSTRUMENT_END ("gegl", "process");
+    }
 
   return object;
 }
diff --git a/gegl/process/gegl-graph-traversal-debug.c b/gegl/process/gegl-graph-traversal-debug.c
index b7b54554e..1ecf7c574 100644
--- a/gegl/process/gegl-graph-traversal-debug.c
+++ b/gegl/process/gegl-graph-traversal-debug.c
@@ -42,7 +42,7 @@ gegl_graph_dump_outputs (GeglNode *node)
   GeglGraphTraversal *path      = gegl_graph_build (node);
   GList              *list_iter = NULL;
 
-  gegl_graph_prepare (path);
+  gegl_graph_prepare (path, NULL);
 
   for (list_iter = g_queue_peek_head_link (&path->path);
        list_iter;
@@ -77,7 +77,7 @@ gegl_graph_dump_request (GeglNode            *node,
   GeglGraphTraversal *path      = gegl_graph_build (node);
   GList              *list_iter = NULL;
 
-  gegl_graph_prepare (path);
+  gegl_graph_prepare (path, NULL);
   gegl_graph_prepare_request (path, roi, 0);
 
   for (list_iter = g_queue_peek_head_link (&path->path);
diff --git a/gegl/process/gegl-graph-traversal.c b/gegl/process/gegl-graph-traversal.c
index af5b32dac..b15ced46f 100644
--- a/gegl/process/gegl-graph-traversal.c
+++ b/gegl/process/gegl-graph-traversal.c
@@ -173,7 +173,8 @@ gegl_graph_get_bounding_box (GeglGraphTraversal *path)
  * Prepare all nodes, initializing their output formats and have rects.
  */
 void
-gegl_graph_prepare (GeglGraphTraversal *path)
+gegl_graph_prepare (GeglGraphTraversal  *path,
+                    GError             **error)
 {
   GList *list_iter = NULL;
 
@@ -190,6 +191,18 @@ gegl_graph_prepare (GeglGraphTraversal *path)
     gegl_operation_prepare (operation);
     node->have_rect = gegl_operation_get_bounding_box (operation);
     node->valid_have_rect = TRUE;
+    if (operation->node->error)
+      {
+        if (error && operation->node->error != *error)
+          {
+            /* Make sure the error is propagated to the calling node error. */
+            g_clear_error (error);
+            g_propagate_error (error, operation->node->error);
+            operation->node->error = NULL;
+          }
+        g_mutex_unlock (&node->mutex);
+        break;
+      }
 
     if (node->cache)
       {
diff --git a/gegl/process/gegl-graph-traversal.h b/gegl/process/gegl-graph-traversal.h
index c7eb658cd..423d35a24 100644
--- a/gegl/process/gegl-graph-traversal.h
+++ b/gegl/process/gegl-graph-traversal.h
@@ -25,7 +25,8 @@ void                gegl_graph_rebuild          (GeglGraphTraversal  *path,
                                                  GeglNode            *node);
 void                gegl_graph_free             (GeglGraphTraversal  *path);
 
-void                gegl_graph_prepare          (GeglGraphTraversal  *path);
+void                gegl_graph_prepare          (GeglGraphTraversal  *path,
+                                                 GError             **error);
 void                gegl_graph_prepare_request  (GeglGraphTraversal  *path,
                                                  const GeglRectangle *roi,
                                                  gint                 level);
diff --git a/gegl/process/gegl-processor.c b/gegl/process/gegl-processor.c
index dcdbe936a..66d791f73 100644
--- a/gegl/process/gegl-processor.c
+++ b/gegl/process/gegl-processor.c
@@ -288,6 +288,14 @@ gegl_processor_set_node (GeglProcessor *processor,
 
   /* Prepare the graph */
   gegl_node_get_bounding_box (processor->input);
+  if (processor->input->error)
+    {
+      if (processor->input != processor->real_node)
+        {
+          g_propagate_error (&processor->real_node->error, processor->input->error);
+          processor->input->error = NULL;
+        }
+    }
 
   g_object_ref (processor->input);
 
diff --git a/operations/external/png-load.c b/operations/external/png-load.c
index 2ef82bc22..d9c754fb0 100644
--- a/operations/external/png-load.c
+++ b/operations/external/png-load.c
@@ -39,13 +39,6 @@ property_uri (uri, _("URI"), "")
 #include <png.h>
 
 
-#define WARN_IF_ERROR(gerror) \
-do { \
-    if (gerror) { \
-      g_warning("gegl:png-load %s", gerror->message); \
-    } \
-} while(0)
-
 typedef enum {
   LOAD_PNG_TOO_SHORT,
   LOAD_PNG_WRONG_HEADER
@@ -477,33 +470,49 @@ static gint query_png (GInputStream *stream,
 static GeglRectangle
 get_bounding_box (GeglOperation *operation)
 {
-  GeglProperties   *o = GEGL_PROPERTIES (operation);
-  GeglRectangle result = {0,0,0,0};
-  gint          width, height;
-  gint          status;
-  const Babl *  format;
-  GError *err = NULL;
-  GFile *infile = NULL;
-
-  GInputStream *stream = gegl_gio_open_input_stream(o->uri, o->path, &infile, &err);
-  WARN_IF_ERROR(err);
-  if (!stream) return result;
-  status = query_png(stream, &width, &height, &format, &err);
-  WARN_IF_ERROR(err);
-  g_input_stream_close(stream, NULL, NULL);
+  GeglProperties *o = GEGL_PROPERTIES (operation);
+  GeglRectangle   result = {0,0,0,0};
+  gint            width, height;
+  gint            status;
+  const Babl     *format;
+  GError         *err    = NULL;
+  GFile          *infile = NULL;
+  GInputStream   *stream;
+
+  stream = gegl_gio_open_input_stream (o->uri, o->path, &infile, &err);
+  if (stream)
+    {
+      status = query_png (stream, &width, &height, &format, &err);
+      g_input_stream_close (stream, NULL, NULL);
+    }
 
-  if (status)
+  if (stream && ! err)
+    {
+      if (status)
+        {
+          width = 0;
+          height = 0;
+        }
+    }
+  else
     {
-      width = 0;
-      height = 0;
+      g_prefix_error (&err, "failed to read file '%s': ", o->path);
+      gegl_operation_set_error (operation, err);
+
+      g_clear_object (&infile);
+      if (stream)
+        g_object_unref (stream);
+
+      return result;
     }
 
   gegl_operation_set_format (operation, "output", format);
   result.width  = width;
-  result.height  = height;
+  result.height = height;
+
+  g_clear_object (&infile);
+  g_object_unref (stream);
 
-  g_clear_object(&infile);
-  g_object_unref(stream);
   return result;
 }
 
@@ -514,28 +523,41 @@ process (GeglOperation       *operation,
          gint                 level)
 {
   GeglProperties *o = GEGL_PROPERTIES (operation);
-  gint        problem;
-  gint        width, height;
-  Babl        *format = NULL;
-  GError *err = NULL;
-  GFile *infile = NULL;
-  GInputStream *stream = gegl_gio_open_input_stream(o->uri, o->path, &infile, &err);
-  WARN_IF_ERROR(err);
-  problem = gegl_buffer_import_png (output, stream, 0, 0,
-                                    &width, &height, format, &err);
-  WARN_IF_ERROR(err);
-  g_input_stream_close(stream, NULL, NULL);
+  gint            width, height;
+  Babl           *format  = NULL;
+  GError         *err     = NULL;
+  GFile          *infile  = NULL;
+  gint            problem = 0;
+  GInputStream   *stream;
+
+  stream = gegl_gio_open_input_stream (o->uri, o->path, &infile, &err);
+  if (stream)
+    {
+      problem = gegl_buffer_import_png (output, stream, 0, 0,
+                                        &width, &height, format, &err);
+      if (err)
+        gegl_operation_set_error (operation, err);
+      g_input_stream_close (stream, NULL, NULL);
+    }
 
   if (problem)
     {
-      g_object_unref(infile);
-      g_object_unref(stream);
-      g_warning ("%s failed to open file %s for reading.",
-                 G_OBJECT_TYPE_NAME (operation), o->path);
+      if (err)
+        g_prefix_error (&err, "failed to read file '%s': ", o->path);
+      else
+        err = g_error_new (g_quark_from_static_string ("gegl"),
+                           0, "failed to read file '%s'.", o->path);
+      gegl_operation_set_error (operation, err);
+
+      g_object_unref (infile);
+      g_object_unref (stream);
+
       return FALSE;
     }
-  g_clear_object(&infile);
-  g_object_unref(stream);
+
+  g_clear_object (&infile);
+  g_object_unref (stream);
+
   return TRUE;
 }
 
diff --git a/tests/simple/test-errors.c b/tests/simple/test-errors.c
index a967153f0..85c627c7d 100644
--- a/tests/simple/test-errors.c
+++ b/tests/simple/test-errors.c
@@ -86,6 +86,115 @@ save_denied (void)
   return success;
 }
 
+/* Trying to load a non-readable file with gegl_node_process(). */
+static gboolean
+load_denied (void)
+{
+  GeglNode   *graph;
+  GeglNode   *source;
+  GeglNode   *sink;
+  GeglBuffer *buffer  = NULL;
+  GError     *error   = NULL;
+  gchar      *path;
+  gboolean    success = FALSE;
+  gint        fd;
+
+  /* Create a new empty file. It is not a valid image but we don't care as we
+   * are going to make it non-readable anyway.
+   */
+  fd = g_file_open_tmp (NULL, &path, NULL);
+  close (fd);
+  g_chmod (path, 0);
+
+  /* Try to load it in a buffer. */
+  graph = gegl_node_new ();
+  source = gegl_node_new_child (graph,
+                                "operation", "gegl:png-load",
+                                "path",      path,
+                                NULL);
+  sink = gegl_node_new_child (graph,
+                              "operation", "gegl:buffer-sink",
+                              "buffer",    &buffer,
+                              NULL);
+  gegl_node_link (source, sink);
+
+  gegl_node_process (sink);
+  if (! gegl_node_process_success (sink, &error))
+    {
+      /* Expected error is "Error opening file “/tmp/.ZBD4YZ”: Permission denied" */
+      success = (error                       &&
+                 error->domain == G_IO_ERROR &&
+                 error->code == G_IO_ERROR_PERMISSION_DENIED);
+    }
+
+  g_object_unref (graph);
+  g_clear_error (&error);
+  if (buffer)
+    g_object_unref (buffer);
+
+  /* Delete the temp file. */
+  g_unlink (path);
+  g_free (path);
+
+  return success;
+}
+
+/* Trying to load an empty file (i.e. not valid PNG) with
+ * gegl_node_blit_buffer(). */
+static gboolean
+load_zero_blit (void)
+{
+  GeglNode   *graph;
+  GeglNode   *source;
+  GeglNode   *scale;
+  GeglBuffer *buffer  = NULL;
+  GError     *error   = NULL;
+  gchar      *path;
+  gboolean    success = FALSE;
+  gint        fd;
+
+  /* Create a new empty file. It is not a valid PNG image. */
+  fd = g_file_open_tmp (NULL, &path, NULL);
+  close (fd);
+
+  /* Try to load it in a buffer. */
+  graph = gegl_node_new ();
+  source = gegl_node_new_child (graph,
+                                "operation", "gegl:png-load",
+                                "path",      path,
+                                NULL);
+  scale = gegl_node_new_child (graph,
+                               "operation", "gegl:scale-ratio",
+                               "x",    2.0,
+                               "y",    2.0,
+                               NULL);
+  gegl_node_link (source, scale);
+  gegl_node_blit_buffer (scale,
+                         buffer,
+                         NULL,
+                         0,
+                         GEGL_ABYSS_NONE);
+
+  if (! gegl_node_process_success (scale, &error))
+    {
+      /* Expected error: "too short for a png file, only 0 bytes." */
+      success = (error &&
+                 error->domain == g_quark_from_static_string ("gegl:load-png-error-quark") &&
+                 error->code == 0);
+    }
+
+  g_object_unref (graph);
+  g_clear_error (&error);
+  if (buffer)
+    g_object_unref (buffer);
+
+  /* Delete the temp file. */
+  g_unlink (path);
+  g_free (path);
+
+  return success;
+}
+
 int
 main (int argc, char **argv)
 {
@@ -97,7 +206,7 @@ main (int argc, char **argv)
                 "use-opencl", FALSE,
                 NULL);
 
-  if (save_denied ())
+  if (save_denied () && load_denied () && load_zero_blit ())
     success = 0;
   else
     success = -1;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]