libsysprof-capture: Drop GError usage from SysprofCaptureWriter

Use `errno` instead, which is icky, but given that all of the failure
modes are from POSIX I/O functions, it’s at least in keeping with them.

This is a major API break.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Helps: #40

src/libsysprof-capture/sysprof-capture-writer-cat.c

@@ -57,6 +57,7 @@
 #include "config.h"
 
 #include <assert.h>
+#include <errno.h>
 #include <glib/gstdio.h>
 #include <stdbool.h>
 #include <stdint.h>
@@ -169,9 +170,8 @@ translate_table_translate (TranslateTable *tables,
 }
 
 bool
-sysprof_capture_writer_cat (SysprofCaptureWriter  *self,
+sysprof_capture_writer_cat (SysprofCaptureWriter *self,
-                            SysprofCaptureReader  *reader,
+                            SysprofCaptureReader *reader)
-                            GError               **error)
 {
   TranslateTable tables[N_TRANSLATE] = { 0, };
   SysprofCaptureFrameType type;
@@ -552,13 +552,9 @@ sysprof_capture_writer_cat (SysprofCaptureWriter  *self,
   return true;
 
 panic:
-  g_set_error (error,
-               G_FILE_ERROR,
-               G_FILE_ERROR_FAILED,
-               "Failed to write data");
-
   translate_table_clear (tables, TRANSLATE_ADDR);
   translate_table_clear (tables, TRANSLATE_CTR);
 
+  errno = EIO;
   return false;
 }

src/libsysprof-capture/sysprof-capture-writer.c

@@ -918,24 +918,26 @@ sysprof_capture_writer_flush (SysprofCaptureWriter *self)
  * sysprof_capture_writer_save_as:
  * @self: A #SysprofCaptureWriter
  * @filename: the file to save the capture as
- * @error: a location for a #GError or %NULL.
  *
  * Saves the captured data as the file @filename.
  *
  * This is primarily useful if the writer was created with a memory-backed
  * file-descriptor such as a memfd or tmpfs file on Linux.
  *
- * Returns: %TRUE if successful, otherwise %FALSE and @error is set.
+ * `errno` is set on error, to any of the errors returned by `open()`,
+ * sysprof_capture_writer_flush(), `lseek()` or `sendfile()`.
+ *
+ * Returns: %TRUE if successful, otherwise %FALSE and `errno` is set.
  */
 bool
-sysprof_capture_writer_save_as (SysprofCaptureWriter  *self,
+sysprof_capture_writer_save_as (SysprofCaptureWriter *self,
-                                const char            *filename,
+                                const char           *filename)
-                                GError               **error)
 {
   size_t to_write;
   off_t in_off;
   off_t pos;
   int fd = -1;
+  int errsv;
 
   assert (self != NULL);
   assert (self->fd != -1);
@@ -975,10 +977,7 @@ sysprof_capture_writer_save_as (SysprofCaptureWriter  *self,
   return true;
 
 handle_errno:
-  g_set_error (error,
+  errsv = errno;
-               G_FILE_ERROR,
-               g_file_error_from_errno (errno),
-               "%s", g_strerror (errno));
 
   if (fd != -1)
     {
@@ -986,6 +985,8 @@ handle_errno:
       unlink (filename);
     }
 
+  errno = errsv;
+
   return false;
 }
 
@@ -993,7 +994,6 @@ handle_errno:
  * _sysprof_capture_writer_splice_from_fd:
  * @self: An #SysprofCaptureWriter
  * @fd: the fd to read from.
- * @error: A location for a #GError, or %NULL.
  *
  * This is internal API for SysprofCaptureWriter and SysprofCaptureReader to
  * communicate when splicing a reader into a writer.
@@ -1003,12 +1003,14 @@ handle_errno:
  *
  * This will not advance the position of @fd.
  *
- * Returns: %TRUE if successful; otherwise %FALSE and @error is set.
+ * `errno` is set on error, to any of the errors returned by `fstat()` or
+ * `sendfile()`, or `EBADMSG` if the file is corrupt.
+ *
+ * Returns: %TRUE if successful; otherwise %FALSE and `errno` is set.
  */
 bool
-_sysprof_capture_writer_splice_from_fd (SysprofCaptureWriter  *self,
+_sysprof_capture_writer_splice_from_fd (SysprofCaptureWriter *self,
-                                        int                    fd,
+                                        int                   fd)
-                                        GError               **error)
 {
   struct stat stbuf;
   off_t in_off;
@@ -1022,10 +1024,7 @@ _sysprof_capture_writer_splice_from_fd (SysprofCaptureWriter  *self,
 
   if (stbuf.st_size < 256)
     {
-      g_set_error (error,
+      errno = EBADMSG;
-                   G_FILE_ERROR,
-                   G_FILE_ERROR_INVAL,
-                   "Cannot splice, possibly corrupt file.");
       return false;
     }
 
@@ -1052,11 +1051,7 @@ _sysprof_capture_writer_splice_from_fd (SysprofCaptureWriter  *self,
   return true;
 
 handle_errno:
-  g_set_error (error,
+  /* errno is propagated */
-               G_FILE_ERROR,
-               g_file_error_from_errno (errno),
-               "%s", g_strerror (errno));
-
   return false;
 }
 
@@ -1064,22 +1059,25 @@ handle_errno:
  * sysprof_capture_writer_splice:
  * @self: An #SysprofCaptureWriter
  * @dest: An #SysprofCaptureWriter
- * @error: A location for a #GError, or %NULL.
  *
  * This function will copy the capture @self into the capture @dest.  This
  * tries to be semi-efficient by using sendfile() to copy the contents between
  * the captures. @self and @dest will be flushed before the contents are copied
  * into the @dest file-descriptor.
  *
- * Returns: %TRUE if successful, otherwise %FALSE and and @error is set.
+ * `errno` is set on error, to any of the errors returned by
+ * sysprof_capture_writer_flush(), `lseek()` or
+ * _sysprof_capture_writer_splice_from_fd().
+ *
+ * Returns: %TRUE if successful, otherwise %FALSE and and `errno` is set.
  */
 bool
-sysprof_capture_writer_splice (SysprofCaptureWriter  *self,
+sysprof_capture_writer_splice (SysprofCaptureWriter *self,
-                               SysprofCaptureWriter  *dest,
+                               SysprofCaptureWriter *dest)
-                               GError               **error)
 {
   bool ret;
   off_t pos;
+  int errsv;
 
   assert (self != NULL);
   assert (self->fd != -1);
@@ -1095,30 +1093,25 @@ sysprof_capture_writer_splice (SysprofCaptureWriter  *self,
     goto handle_errno;
 
   /* Perform the splice */
-  ret = _sysprof_capture_writer_splice_from_fd (dest, self->fd, error);
+  ret = _sysprof_capture_writer_splice_from_fd (dest, self->fd);
+  errsv = errno;
 
   /* Now reset or file-descriptor position (it should be the same */
   if (pos != lseek (self->fd, pos, SEEK_SET))
-    {
+    goto handle_errno;
-      ret = false;
-      goto handle_errno;
-    }
 
+  if (!ret)
+    errno = errsv;
   return ret;
 
 handle_errno:
-  g_set_error (error,
+  /* errno is propagated */
-               G_FILE_ERROR,
-               g_file_error_from_errno (errno),
-               "%s", g_strerror (errno));
-
   return false;
 }
 
 /**
  * sysprof_capture_writer_create_reader:
  * @self: A #SysprofCaptureWriter
- * @error: a location for a #GError, or %NULL
  *
  * Creates a new reader for the writer.
  *
@@ -1127,11 +1120,14 @@ handle_errno:
  * also consuming from the reader, you could get transient failures unless you
  * synchronize the operations.
  *
+ * `errno` is set on error, to any of the errors returned by
+ * sysprof_capture_writer_flush(), `dup()` or
+ * sysprof_capture_reader_new_from_fd().
+ *
  * Returns: (transfer full): A #SysprofCaptureReader.
  */
 SysprofCaptureReader *
-sysprof_capture_writer_create_reader (SysprofCaptureWriter  *self,
+sysprof_capture_writer_create_reader (SysprofCaptureWriter *self)
-                                      GError               **error)
 {
   SysprofCaptureReader *ret;
   int copy;
@@ -1141,10 +1137,7 @@ sysprof_capture_writer_create_reader (SysprofCaptureWriter  *self,
 
   if (!sysprof_capture_writer_flush (self))
     {
-      g_set_error (error,
+      /* errno is propagated */
-                   G_FILE_ERROR,
-                   g_file_error_from_errno (errno),
-                   "%s", g_strerror (errno));
       return NULL;
     }
 
@@ -1153,10 +1146,18 @@ sysprof_capture_writer_create_reader (SysprofCaptureWriter  *self,
    * uses positioned reads.
    */
   if (-1 == (copy = dup (self->fd)))
-    return NULL;
+    {
+      /* errno is propagated */
+      return NULL;
+    }
 
-  if ((ret = sysprof_capture_reader_new_from_fd (copy, error)))
+  if (!(ret = sysprof_capture_reader_new_from_fd (copy)))
-    sysprof_capture_reader_set_stat (ret, &self->stat);
+    {
+      /* errno is propagated */
+      return NULL;
+    }
+
+  sysprof_capture_reader_set_stat (ret, &self->stat);
 
   return sysprof_steal_pointer (&ret);
 }

src/libsysprof-capture/sysprof-capture-writer.h

@@ -207,29 +207,24 @@ SYSPROF_AVAILABLE_IN_ALL
 bool                  sysprof_capture_writer_flush                           (SysprofCaptureWriter              *self);
 SYSPROF_AVAILABLE_IN_ALL
 bool                  sysprof_capture_writer_save_as                         (SysprofCaptureWriter              *self,
-                                                                              const char                        *filename,
+                                                                              const char                        *filename);
-                                                                              GError                           **error);
 SYSPROF_AVAILABLE_IN_ALL
 unsigned int          sysprof_capture_writer_request_counter                 (SysprofCaptureWriter              *self,
                                                                               unsigned int                       n_counters);
 SYSPROF_AVAILABLE_IN_ALL
-SysprofCaptureReader *sysprof_capture_writer_create_reader                   (SysprofCaptureWriter              *self,
+SysprofCaptureReader *sysprof_capture_writer_create_reader                   (SysprofCaptureWriter              *self);
-                                                                              GError                           **error);
 SYSPROF_AVAILABLE_IN_ALL
 bool                  sysprof_capture_writer_splice                          (SysprofCaptureWriter              *self,
-                                                                              SysprofCaptureWriter              *dest,
+                                                                              SysprofCaptureWriter              *dest);
-                                                                              GError                           **error);
 SYSPROF_AVAILABLE_IN_ALL
 bool                  sysprof_capture_writer_cat                             (SysprofCaptureWriter              *self,
-                                                                              SysprofCaptureReader              *reader,
+                                                                              SysprofCaptureReader              *reader);
-                                                                              GError                           **error);
 SYSPROF_INTERNAL
 bool                  _sysprof_capture_writer_add_raw                        (SysprofCaptureWriter              *self,
                                                                               const SysprofCaptureFrame         *frame);
 SYSPROF_INTERNAL
 bool                  _sysprof_capture_writer_splice_from_fd                 (SysprofCaptureWriter              *self,
-                                                                              int                                fd,
+                                                                              int                                fd) SYSPROF_INTERNAL;
-                                                                              GError                           **error) SYSPROF_INTERNAL;
 SYSPROF_INTERNAL
 bool                  _sysprof_capture_writer_set_time_range                 (SysprofCaptureWriter              *self,
                                                                               int64_t                            start_time,

src/libsysprof/sysprof-local-profiler.c

@@ -198,7 +198,8 @@ sysprof_local_profiler_finish_stopping (SysprofLocalProfiler *self)
   g_assert (priv->is_stopping == TRUE);
   g_assert (priv->stopping->len == 0);
 
-  reader = sysprof_capture_writer_create_reader (priv->writer, 0);
+  reader = sysprof_capture_writer_create_reader (priv->writer);
+  g_assert (reader != NULL);
 
   for (guint i = 0; i < priv->sources->len; i++)
     {

src/libsysprof/sysprof-proxy-source.c

@@ -450,12 +450,11 @@ sysprof_proxy_source_cat (SysprofProxySource   *self,
   g_assert (SYSPROF_IS_PROXY_SOURCE (self));
   g_assert (reader != NULL);
 
-  if (self->writer != NULL)
+  if (self->writer != NULL &&
+      !sysprof_capture_writer_cat (self->writer, reader))
     {
-      g_autoptr(GError) error = NULL;
+      int errsv = errno;
-
+      g_warning ("Failed to join reader: %s", g_strerror (errsv));
-      if (!sysprof_capture_writer_cat (self->writer, reader, &error))
-        g_warning ("Failed to join reader: %s\n", error->message);
     }
 }
 

src/libsysprof/sysprof-tracefd-source.c

@@ -259,8 +259,9 @@ sysprof_tracefd_source_stop (SysprofSource *source)
     {
       g_autoptr(SysprofCaptureReader) reader = NULL;
 
+      /* FIXME: Error handling is ignored here */
       if ((reader = sysprof_capture_reader_new_from_fd (priv->tracefd)))
-        sysprof_capture_writer_cat (priv->writer, reader, NULL);
+        sysprof_capture_writer_cat (priv->writer, reader);
 
       priv->tracefd = -1;
     }

src/tests/test-capture.c

@@ -399,9 +399,8 @@ test_reader_basic (void)
       g_assert_not_reached ();
   }
 
-  r = sysprof_capture_writer_save_as (writer, "capture-file.bak", &error);
+  r = sysprof_capture_writer_save_as (writer, "capture-file.bak");
-  g_assert_no_error (error);
+  g_assert_true (r);
-  g_assert (r);
   g_assert (g_file_test ("capture-file.bak", G_FILE_TEST_IS_REGULAR));
 
   /* make sure contents are equal */
@@ -462,7 +461,6 @@ test_writer_splice (void)
   SysprofCaptureWriter *writer2;
   SysprofCaptureReader *reader;
   SysprofCaptureFrameType type;
-  GError *error = NULL;
   guint i;
   gint r;
 
@@ -472,9 +470,8 @@ test_writer_splice (void)
   for (i = 0; i < 1000; i++)
     sysprof_capture_writer_add_timestamp (writer1, SYSPROF_CAPTURE_CURRENT_TIME, -1, -1);
 
-  r = sysprof_capture_writer_splice (writer1, writer2, &error);
+  r = sysprof_capture_writer_splice (writer1, writer2);
-  g_assert_no_error (error);
+  g_assert_true (r);
-  g_assert (r == TRUE);
 
   g_clear_pointer (&writer1, sysprof_capture_writer_unref);
   g_clear_pointer (&writer2, sysprof_capture_writer_unref);
@@ -815,7 +812,6 @@ test_reader_writer_cat_jitmap (void)
   SysprofCaptureWriter *res;
   SysprofCaptureReader *reader;
   const SysprofCaptureSample *sample;
-  GError *error = NULL;
   SysprofCaptureAddress addrs[20];
   gboolean r;
 
@@ -851,26 +847,21 @@ test_reader_writer_cat_jitmap (void)
                                      addrs,
                                      G_N_ELEMENTS (addrs));
 
-  reader = sysprof_capture_writer_create_reader (writer1, &error);
+  reader = sysprof_capture_writer_create_reader (writer1);
-  g_assert_no_error (error);
   g_assert_nonnull (reader);
-  r = sysprof_capture_writer_cat (res, reader, &error);
+  r = sysprof_capture_writer_cat (res, reader);
-  g_assert_no_error (error);
   g_assert_true (r);
   sysprof_capture_writer_unref (writer1);
   sysprof_capture_reader_unref (reader);
 
-  reader = sysprof_capture_writer_create_reader (writer2, &error);
+  reader = sysprof_capture_writer_create_reader (writer2);
-  g_assert_no_error (error);
   g_assert_nonnull (reader);
-  r = sysprof_capture_writer_cat (res, reader, &error);
+  r = sysprof_capture_writer_cat (res, reader);
-  g_assert_no_error (error);
   g_assert_true (r);
   sysprof_capture_writer_unref (writer2);
   sysprof_capture_reader_unref (reader);
 
-  reader = sysprof_capture_writer_create_reader (res, &error);
+  reader = sysprof_capture_writer_create_reader (res);
-  g_assert_no_error (error);
   g_assert_nonnull (reader);
   sysprof_capture_reader_read_jitmap (reader);
   sample = sysprof_capture_reader_read_sample (reader);
@@ -891,7 +882,6 @@ test_writer_memory_alloc_free (void)
 {
   SysprofCaptureWriter *writer;
   SysprofCaptureReader *reader;
-  GError *error = NULL;
   SysprofCaptureAddress addrs[20] = {
     0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
     11, 12, 13, 14, 15, 16, 17, 18, 19,
@@ -916,8 +906,7 @@ test_writer_memory_alloc_free (void)
 
   sysprof_capture_writer_flush (writer);
 
-  reader = sysprof_capture_writer_create_reader (writer, &error);
+  reader = sysprof_capture_writer_create_reader (writer);
-  g_assert_no_error (error);
   g_assert_nonnull (reader);
 
   for (guint i = 0; i < 20; i++)

src/tools/sysprof-cat.c

@@ -81,10 +81,11 @@ main (gint   argc,
           return EXIT_FAILURE;
         }
 
-      if (!sysprof_capture_writer_cat (writer, reader, &error))
+      if (!sysprof_capture_writer_cat (writer, reader))
         {
+          int errsv = errno;
           g_printerr ("Failed to join \"%s\": %s\n",
-                      argv[i], error->message);
+                      argv[i], g_strerror (errsv));
           return EXIT_FAILURE;
         }
     }

src/tools/sysprof-cli.c

@@ -134,7 +134,6 @@ merge_files (gint             argc,
   for (guint i = 1; i < argc; i++)
     {
       g_autoptr(SysprofCaptureReader) reader = NULL;
-      g_autoptr(GError) error = NULL;
 
       if (!(reader = sysprof_capture_reader_new (argv[i])))
         {
@@ -144,10 +143,11 @@ merge_files (gint             argc,
           return EXIT_FAILURE;
         }
 
-      if (!sysprof_capture_writer_cat (writer, reader, &error))
+      if (!sysprof_capture_writer_cat (writer, reader))
         {
+          int errsv = errno;
           g_printerr ("Failed to join \"%s\": %s\n",
-                      argv[i], error->message);
+                      argv[i], g_strerror (errsv));
           return EXIT_FAILURE;
         }
     }