rust: Rework treefile to be an object

In a later patch I'm going to add more API; basically rather
than doing the JSON parsing from C, we can add APIs to directly
access the treefile object.  This also demonstrates how we
can do more extensive APIs, in particular implement an "object"
in Rust.

The ownership across the FFI boundary becomes nicer here too,
we don't need to do a dance with the fd.

For writing this I found
http://jakegoulding.com/rust-ffi-omnibus/objects/
quite useful, as well as
https://github.com/rust-lang/regex/blob/master/regex-capi/src/rure.rs

Closes: #1474
Approved by: jlebon

rust/Cargo.toml

@@ -12,6 +12,7 @@ libc = "0.2"
 glib-sys = "0.6.0"
 gio-sys = "0.6.0"
 glib = "0.5.0"
+tempfile = "3.0.3"
 
 [lib]
 name = "rpmostree_rust"

rust/include/rpmostree-rust.h

@@ -20,6 +20,13 @@
 
 #pragma once
 
-int rpmostree_rs_treefile_read (const char *filename,
-                                const char *arch,
-                                int output_fd, GError **error);
+typedef struct RpmOstreeRsTreefile RpmOstreeRsTreefile;
+
+RpmOstreeRsTreefile *rpmostree_rs_treefile_new (const char *filename,
+                                                const char *arch,
+                                                GError **error);
+
+int rpmostree_rs_treefile_to_json (RpmOstreeRsTreefile *tf, GError **error);
+
+void rpmostree_rs_treefile_free (RpmOstreeRsTreefile *tf);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(RpmOstreeRsTreefile, rpmostree_rs_treefile_free);

rust/src/glibutils.rs

@@ -34,7 +34,7 @@ use std::ptr;
 // return a Result (using the std Error).
 // TODO: Try upstreaming this into the glib crate?
 
-fn error_to_glib(e: &Error, gerror: *mut *mut glib_sys::GError) {
+pub fn error_to_glib(e: &Error, gerror: *mut *mut glib_sys::GError) {
     if gerror.is_null() {
         return;
     }
@@ -49,6 +49,7 @@ fn error_to_glib(e: &Error, gerror: *mut *mut glib_sys::GError) {
     }
 }
 
+#[allow(dead_code)]
 pub fn int_glib_error<T, E>(res: Result<T, E>, gerror: *mut *mut glib_sys::GError) -> libc::c_int
 where
     E: std::error::Error,

rust/src/lib.rs

@@ -20,6 +20,7 @@ extern crate gio_sys;
 extern crate glib;
 extern crate glib_sys;
 extern crate libc;
+extern crate tempfile;
 
 #[macro_use]
 extern crate serde_derive;
@@ -29,13 +30,12 @@ extern crate serde_yaml;
 
 use std::ffi::{CStr, OsStr};
 use std::os::unix::ffi::OsStrExt;
-use std::os::unix::io::{FromRawFd, IntoRawFd};
-use std::{fs, io};
+use std::os::unix::io::IntoRawFd;
 
 mod glibutils;
 use glibutils::*;
 mod treefile;
-use treefile::treefile_read_impl;
+use treefile::*;
 
 /* Wrapper functions for translating from C to Rust */
 
@@ -58,23 +58,41 @@ fn bytes_from_nonnull<'a>(s: *const libc::c_char) -> &'a [u8] {
 }
 
 #[no_mangle]
-pub extern "C" fn rpmostree_rs_treefile_read(
+pub extern "C" fn rpmostree_rs_treefile_new(
     filename: *const libc::c_char,
     arch: *const libc::c_char,
-    fd: libc::c_int,
     error: *mut *mut glib_sys::GError,
-) -> libc::c_int {
+) -> *mut Treefile {
     // Convert arguments
     let filename = OsStr::from_bytes(bytes_from_nonnull(filename));
     let arch = str_from_nullable(arch);
-    // Using an O_TMPFILE is an easy way to avoid ownership transfer issues w/
-    // returning allocated memory across the Rust/C boundary; the dance with
-    // `file` is to avoid dup()ing the fd unnecessarily.
-    let file = unsafe { fs::File::from_raw_fd(fd) };
-    let r = {
-        let output = io::BufWriter::new(&file);
-        int_glib_error(treefile_read_impl(filename.as_ref(), arch, output), error)
-    };
-    file.into_raw_fd(); // Drop ownership of the FD again
-    r
+    // Run code, map error if any, otherwise extract raw pointer, passing
+    // ownership back to C.
+    ptr_glib_error(Treefile::new_boxed(filename.as_ref(), arch), error)
+}
+
+#[no_mangle]
+pub extern "C" fn rpmostree_rs_treefile_to_json(
+    tf: *mut Treefile,
+    gerror: *mut *mut glib_sys::GError,
+) -> libc::c_int {
+    assert!(!tf.is_null());
+    let tf = unsafe { &mut *tf };
+    match tf.serialize_json_fd() {
+        Ok(f) => f.into_raw_fd() as libc::c_int,
+        Err(e) => {
+            error_to_glib(&e, gerror);
+            -1 as libc::c_int
+        }
+    }
+}
+
+#[no_mangle]
+pub extern "C" fn rpmostree_rs_treefile_free(tf: *mut Treefile) {
+    if tf.is_null() {
+        return;
+    }
+    unsafe {
+        Box::from_raw(tf);
+    }
 }

rust/src/treefile.rs

@@ -22,12 +22,17 @@
 
 use serde_json;
 use serde_yaml;
-
+use std::io::prelude::*;
 use std::path::Path;
 use std::{fs, io};
+use tempfile;
 
 const ARCH_X86_64: &'static str = "x86_64";
 
+pub struct Treefile {
+    pub parsed: TreeComposeConfig,
+}
+
 /// Parse a YAML treefile definition using architecture `arch`.
 fn treefile_parse_yaml<R: io::Read>(input: R, arch: Option<&str>) -> io::Result<TreeComposeConfig> {
     let mut treefile: TreeComposeConfig = match serde_yaml::from_reader(input) {
@@ -77,15 +82,22 @@ fn treefile_parse_yaml<R: io::Read>(input: R, arch: Option<&str>) -> io::Result<
     Ok(treefile)
 }
 
-pub fn treefile_read_impl<W: io::Write>(
-    filename: &Path,
-    arch: Option<&str>,
-    output: W,
-) -> io::Result<()> {
-    let f = io::BufReader::new(fs::File::open(filename)?);
-    let treefile = treefile_parse_yaml(f, arch)?;
-    serde_json::to_writer_pretty(output, &treefile)?;
-    Ok(())
+impl Treefile {
+    pub fn new_boxed(filename: &Path, arch: Option<&str>) -> io::Result<Box<Treefile>> {
+        let f = io::BufReader::new(fs::File::open(filename)?);
+        let parsed = treefile_parse_yaml(f, arch)?;
+        Ok(Box::new(Treefile { parsed: parsed }))
+    }
+
+    pub fn serialize_json_fd(&self) -> io::Result<fs::File> {
+        let mut tmpf = tempfile::tempfile()?;
+        {
+            let output = io::BufWriter::new(&tmpf);
+            serde_json::to_writer_pretty(output, &self.parsed)?;
+        }
+        tmpf.seek(io::SeekFrom::Start(0))?;
+        Ok(tmpf)
+    }
 }
 
 fn whitespace_split_packages(pkgs: &[String]) -> Vec<String> {

src/app/rpmostree-compose-builtin-tree.c

@@ -23,6 +23,7 @@
 #include <string.h>
 #include <glib-unix.h>
 #include <json-glib/json-glib.h>
+#include <gio/gunixinputstream.h>
 #include <gio/gunixoutputstream.h>
 #include <libdnf/libdnf.h>
 #include <libdnf/dnf-repo.h>
@@ -125,6 +126,9 @@ typedef struct {
   char *rojig_spec;
   char *previous_checksum;
 
+#ifdef HAVE_RUST
+  RpmOstreeRsTreefile *treefile_rs;
+#endif
   JsonParser *treefile_parser;
   JsonNode *treefile_rootval; /* Unowned */
   JsonObject *treefile; /* Unowned */
@@ -155,6 +159,9 @@ rpm_ostree_tree_compose_context_free (RpmOstreeTreeComposeContext *ctx)
   g_free (ctx->ref);
   g_free (ctx->rojig_spec);
   g_free (ctx->previous_checksum);
+#ifdef HAVE_RUST
+  g_clear_pointer (&ctx->treefile_rs, (GDestroyNotify) rpmostree_rs_treefile_free);
+#endif
   g_clear_object (&ctx->treefile_parser);
   g_clear_pointer (&ctx->serialized_treefile, (GDestroyNotify)g_bytes_unref);
   g_free (ctx);
@@ -682,11 +689,7 @@ parse_treefile_to_json (RpmOstreeTreeComposeContext  *self,
                         JsonParser   **out_parser,
                         GError       **error)
 {
-#ifdef HAVE_RUST
-  g_autofree char *fdpath = NULL;
-  g_auto(GLnxTmpfile) json_contents = { 0, };
-#endif
-
+  g_autoptr(JsonParser) parser = json_parser_new ();
   if (g_str_has_suffix (treefile_path, ".yaml") ||
       g_str_has_suffix (treefile_path, ".yml"))
     {
@@ -694,22 +697,25 @@ parse_treefile_to_json (RpmOstreeTreeComposeContext  *self,
       return glnx_throw (error, "This version of rpm-ostree was built without "
                                 "rust, and doesn't support YAML treefiles");
 #else
-      if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, &json_contents, error))
-        return FALSE;
-
       const char *arch = self ? dnf_context_get_base_arch (rpmostree_context_get_dnf (self->corectx)) : NULL;
-      if (!rpmostree_rs_treefile_read (treefile_path, arch,
-                                       json_contents.fd, error))
+      self->treefile_rs = rpmostree_rs_treefile_new (treefile_path, arch, error);
+      if (!self->treefile_rs)
         return glnx_prefix_error (error, "Failed to load YAML treefile");
 
-      /* or just lseek back to 0 and use json_parser_load_from_data here? */
-      treefile_path = fdpath = g_strdup_printf ("/proc/self/fd/%d", json_contents.fd);
+      glnx_fd_close int json_fd = rpmostree_rs_treefile_to_json (self->treefile_rs, error);
+      if (json_fd < 0)
+        return FALSE;
+      g_autoptr(GInputStream) json_s = g_unix_input_stream_new (json_fd, FALSE);
+
+      if (!json_parser_load_from_stream (parser, json_s, NULL, error))
+        return FALSE;
 #endif
     }
-
-  g_autoptr(JsonParser) parser = json_parser_new ();
-  if (!json_parser_load_from_file (parser, treefile_path, error))
-    return FALSE;
+  else
+    {
+      if (!json_parser_load_from_file (parser, treefile_path, error))
+        return FALSE;
+    }
 
   *out_parser = g_steal_pointer (&parser);
   return TRUE;