[librsvg] rsvg_handle_write()/close(): Port entirely to Rust. Yay!

[Federico Mena Quintero <federico src gnome org>]

commit 437ce5fa90ff272852d0694c96920e571c128b13
Author: Federico Mena Quintero <federico gnome org>
Date:   Tue Dec 18 09:00:28 2018 -0600

    rsvg_handle_write()/close(): Port entirely to Rust.  Yay!

 librsvg/rsvg-handle.c        | 108 +++----------------------------------------
 rsvg_internals/src/error.rs  |   7 ++-
 rsvg_internals/src/handle.rs | 104 +++++++++++++++++++++++++++++++++++++++++
 rsvg_internals/src/lib.rs    |  11 ++---
 rsvg_internals/src/load.rs   | 100 +++++----------------------------------
 rsvg_internals/src/xml.rs    |  31 +------------
 6 files changed, 131 insertions(+), 230 deletions(-)
---
diff --git a/librsvg/rsvg-handle.c b/librsvg/rsvg-handle.c
index 2564f407..8ac1ad8d 100644
--- a/librsvg/rsvg-handle.c
+++ b/librsvg/rsvg-handle.c
@@ -145,7 +145,6 @@ typedef enum {
 } RsvgHandleState;
 
 /* Implemented in rsvg_internals/src/xml.rs */
-extern RsvgXmlState *rsvg_xml_state_new (RsvgHandle *handle);
 extern void rsvg_xml_state_error(RsvgXmlState *xml, const char *msg);
 
 G_GNUC_INTERNAL
@@ -173,16 +172,8 @@ extern gboolean rsvg_handle_rust_read_stream_sync (RsvgHandle *handle,
                                                    GInputStream *stream,
                                                    GCancellable *cancellable,
                                                    GError **error);
-
-/* Implemented in rsvg_internals/src/xml.rs */
-extern void rsvg_xml_state_free (RsvgXmlState *xml);
-extern gboolean rsvg_xml_state_tree_is_valid(RsvgXmlState *xml, GError **error);
-
-/* Implemented in rsvg_internals/src/load.rs */
-extern RsvgLoad *rsvg_load_new (RsvgXmlState *xml, guint flags) G_GNUC_WARN_UNUSED_RESULT;
-extern RsvgXmlState *rsvg_load_free (RsvgLoad *load) G_GNUC_WARN_UNUSED_RESULT;
-extern void rsvg_load_write (RsvgLoad *load, const guchar *buf, gsize count);
-extern gboolean rsvg_load_close (RsvgLoad *load, GError **error) G_GNUC_WARN_UNUSED_RESULT;
+extern void rsvg_handle_rust_write (RsvgHandle *handle, const guchar *buf, gsize count);
+extern gboolean rsvg_handle_rust_close (RsvgHandle *handle, GError **error);
 
 
 /* Implemented in rust/src/node.rs */
@@ -210,8 +201,6 @@ extern void rsvg_drawing_ctx_get_geometry (RsvgDrawingCtx *ctx,
                                            RsvgRectangle *logical_rect);
 
 struct RsvgHandlePrivate {
-    RsvgLoad *load;
-
     RsvgSizeFunc size_func;
     gpointer user_data;
     GDestroyNotify user_data_destroy;
@@ -276,13 +265,6 @@ rsvg_handle_dispose (GObject *instance)
         self->priv->user_data_destroy = NULL;
     }
 
-    if (self->priv->load) {
-        RsvgXmlState *xml = rsvg_load_free (self->priv->load);
-        self->priv->load = NULL;
-
-        rsvg_xml_state_free (xml);
-    }
-
     g_clear_pointer (&self->priv->base_uri, g_free);
 
 #ifdef HAVE_PANGOFT2
@@ -722,54 +704,14 @@ rsvg_handle_new_from_stream_sync (GInputStream   *input_stream,
 gboolean
 rsvg_handle_write (RsvgHandle *handle, const guchar *buf, gsize count, GError **error)
 {
-    RsvgHandlePrivate *priv;
-    RsvgHandleState lstate;
-
+    rsvg_return_val_if_fail (RSVG_IS_HANDLE (handle), FALSE, error);
     g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
-    rsvg_return_val_if_fail (handle, FALSE, error);
-
-    priv = handle->priv;
-
-    lstate = rsvg_handle_rust_get_load_state (priv->rust_handle);
-
-    g_return_val_if_fail (lstate == RSVG_HANDLE_STATE_START
-                          || lstate == RSVG_HANDLE_STATE_LOADING,
-                          FALSE);
-
-    if (lstate == RSVG_HANDLE_STATE_START) {
-        rsvg_handle_rust_set_load_state (priv->rust_handle, RSVG_HANDLE_STATE_LOADING);
-        priv->load = rsvg_load_new (rsvg_xml_state_new (handle),
-                                    rsvg_handle_rust_get_flags (priv->rust_handle));
-    }
-
-    g_assert (rsvg_handle_rust_get_load_state (priv->rust_handle) == RSVG_HANDLE_STATE_LOADING);
-
-    rsvg_load_write (priv->load, buf, count);
+    g_return_val_if_fail ((buf != NULL && count != 0) || (count == 0), FALSE);
 
+    rsvg_handle_rust_write (handle, buf, count);
     return TRUE;
 }
 
-static gboolean
-finish_load (RsvgHandle *handle, RsvgXmlState *xml, gboolean was_successful, GError **error)
-{
-    if (was_successful) {
-        g_assert (error == NULL || *error == NULL);
-
-        was_successful = rsvg_xml_state_tree_is_valid (xml, error);
-        if (was_successful) {
-            rsvg_handle_rust_steal_result (handle->priv->rust_handle, xml);
-        }
-    }
-
-    if (was_successful) {
-        rsvg_handle_rust_set_load_state (handle->priv->rust_handle, RSVG_HANDLE_STATE_CLOSED_OK);
-    } else {
-        rsvg_handle_rust_set_load_state (handle->priv->rust_handle, RSVG_HANDLE_STATE_CLOSED_ERROR);
-    }
-
-    return was_successful;
-}
-
 /**
  * rsvg_handle_close:
  * @handle: a #RsvgHandle
@@ -787,48 +729,10 @@ finish_load (RsvgHandle *handle, RsvgXmlState *xml, gboolean was_successful, GEr
 gboolean
 rsvg_handle_close (RsvgHandle *handle, GError **error)
 {
-    RsvgHandlePrivate *priv;
-    gboolean read_successfully;
-    gboolean result = FALSE;
-    RsvgXmlState *xml;
-
     g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
     rsvg_return_val_if_fail (handle, FALSE, error);
 
-    priv = handle->priv;
-
-    switch (rsvg_handle_rust_get_load_state (priv->rust_handle)) {
-    case RSVG_HANDLE_STATE_START:
-        g_set_error (error, RSVG_ERROR, RSVG_ERROR_FAILED, _("no data passed to parser"));
-        rsvg_handle_rust_set_load_state (priv->rust_handle, RSVG_HANDLE_STATE_CLOSED_ERROR);
-        result = FALSE;
-        break;
-
-    case RSVG_HANDLE_STATE_LOADING:
-        g_assert (priv->load != NULL);
-        read_successfully = rsvg_load_close (priv->load, error);
-
-        xml = rsvg_load_free (priv->load);
-        priv->load = NULL;
-
-        result = finish_load (handle, xml, read_successfully, error);
-        rsvg_xml_state_free (xml);
-        break;
-
-    case RSVG_HANDLE_STATE_CLOSED_OK:
-    case RSVG_HANDLE_STATE_CLOSED_ERROR:
-        /* closing is idempotent */
-        result = TRUE;
-        break;
-
-    default:
-        g_assert_not_reached ();
-    }
-
-    g_assert (rsvg_handle_rust_get_load_state (priv->rust_handle) == RSVG_HANDLE_STATE_CLOSED_OK
-              || rsvg_handle_rust_get_load_state (priv->rust_handle) == RSVG_HANDLE_STATE_CLOSED_ERROR);
-
-    return result;
+    return rsvg_handle_rust_close(handle, error);
 }
 
 /**
diff --git a/rsvg_internals/src/error.rs b/rsvg_internals/src/error.rs
index 72770b6e..5d6856cc 100644
--- a/rsvg_internals/src/error.rs
+++ b/rsvg_internals/src/error.rs
@@ -139,9 +139,10 @@ impl<O, E: Into<ValueErrorKind>> AttributeResultExt<O, E> for Result<O, E> {
 
 #[derive(Debug, Clone)]
 pub enum LoadingError {
+    NoDataPassedToParser,
+    XmlParseError(String),
     // Could not parse data: URL
     CouldNotCreateXmlParser,
-    XmlParseError(String),
     BadDataUrl,
     Cairo(cairo::Status),
     EmptyData,
@@ -154,6 +155,7 @@ pub enum LoadingError {
 impl error::Error for LoadingError {
     fn description(&self) -> &str {
         match *self {
+            LoadingError::NoDataPassedToParser => "no data passed to parser",
             LoadingError::CouldNotCreateXmlParser => "could not create XML parser",
             LoadingError::XmlParseError(_) => "XML parse error",
             LoadingError::BadDataUrl => "invalid data: URL",
@@ -172,7 +174,8 @@ impl fmt::Display for LoadingError {
         match *self {
             LoadingError::Cairo(status) => write!(f, "cairo error: {:?}", status),
             LoadingError::XmlParseError(ref s) => write!(f, "XML parse error: {}", s),
-            LoadingError::CouldNotCreateXmlParser
+            LoadingError::NoDataPassedToParser
+            | LoadingError::CouldNotCreateXmlParser
             | LoadingError::BadDataUrl
             | LoadingError::EmptyData
             | LoadingError::SvgHasNoElements
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index 4d52d503..28857983 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -2,6 +2,7 @@ use std::cell::{Cell, Ref, RefCell};
 use std::error::Error;
 use std::ptr;
 use std::rc::Rc;
+use std::slice;
 
 use cairo::{ImageSurface, Status};
 use cairo_sys;
@@ -19,6 +20,7 @@ use defs::{Fragment, Href};
 use dpi::Dpi;
 use error::{set_gerror, LoadingError};
 use io;
+use load::LoadContext;
 use node::{box_node, Node, RsvgNode};
 use surface_utils::shared_surface::SharedImageSurface;
 use svg::Svg;
@@ -60,6 +62,7 @@ pub struct Handle {
     svg: RefCell<Option<Svg>>,
     load_options: Cell<LoadOptions>,
     load_state: Cell<LoadState>,
+    load: RefCell<Option<LoadContext>>,
 }
 
 impl Handle {
@@ -70,6 +73,7 @@ impl Handle {
             svg: RefCell::new(None),
             load_options: Cell::new(LoadOptions::default()),
             load_state: Cell::new(LoadState::Start),
+            load: RefCell::new(None),
         }
     }
 
@@ -114,6 +118,69 @@ impl Handle {
         *self.svg.borrow_mut() = Some(xml.steal_result());
         Ok(())
     }
+
+    pub fn write(&mut self, handle: *mut RsvgHandle, buf: &[u8]) {
+        assert!(
+            self.load_state.get() == LoadState::Start
+                || self.load_state.get() == LoadState::Loading
+        );
+
+        if self.load_state.get() == LoadState::Start {
+            self.load_state.set(LoadState::Loading);
+
+            self.load = RefCell::new(Some(LoadContext::new(handle, self.load_options.get())));
+        }
+
+        assert!(self.load_state.get() == LoadState::Loading);
+
+        self.load.borrow_mut().as_mut().unwrap().write(buf);
+    }
+
+    pub fn close(&mut self) -> Result<(), LoadingError> {
+        let load_state = self.load_state.get();
+
+        let res = match load_state {
+            LoadState::Start => {
+                self.load_state.set(LoadState::ClosedError);
+                Err(LoadingError::NoDataPassedToParser)
+            }
+
+            LoadState::Loading => self
+                .close_internal()
+                .and_then(|_| {
+                    self.load_state.set(LoadState::ClosedOk);
+                    Ok(())
+                })
+                .map_err(|e| {
+                    self.load_state.set(LoadState::ClosedError);
+                    e
+                }),
+
+            LoadState::ClosedOk | LoadState::ClosedError => {
+                // closing is idempotent
+                Ok(())
+            }
+        };
+
+        assert!(
+            self.load_state.get() == LoadState::ClosedOk
+                || self.load_state.get() == LoadState::ClosedError
+        );
+
+        res
+    }
+
+    fn close_internal(&mut self) -> Result<(), LoadingError> {
+        let mut r = self.load.borrow_mut();
+        let mut load = r.take().unwrap();
+
+        let mut xml = load.close()?;
+
+        xml.validate_tree()?;
+
+        *self.svg.borrow_mut() = Some(xml.steal_result());
+        Ok(())
+    }
 }
 
 // Keep these in sync with rsvg.h:RsvgHandleFlags
@@ -670,3 +737,40 @@ pub unsafe extern "C" fn rsvg_handle_rust_read_stream_sync(
         }
     }
 }
+
+#[no_mangle]
+pub unsafe extern "C" fn rsvg_handle_rust_write(
+    handle: *mut RsvgHandle,
+    buf: *const u8,
+    count: usize,
+) {
+    let rhandle = get_rust_handle(handle);
+
+    let load_state = rhandle.load_state.get();
+
+    if !(load_state == LoadState::Start || load_state == LoadState::Loading) {
+        rsvg_g_warning("handle must not be closed in order to write to it");
+        return;
+    }
+
+    let buffer = slice::from_raw_parts(buf, count);
+
+    rhandle.write(handle, buffer);
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn rsvg_handle_rust_close(
+    handle: *mut RsvgHandle,
+    error: *mut *mut glib_sys::GError,
+) -> glib_sys::gboolean {
+    let rhandle = get_rust_handle(handle);
+
+    match rhandle.close() {
+        Ok(()) => true.to_glib(),
+
+        Err(e) => {
+            set_gerror(error, 0, &format!("{}", e));
+            false.to_glib()
+        }
+    }
+}
diff --git a/rsvg_internals/src/lib.rs b/rsvg_internals/src/lib.rs
index 4ea471df..bec58f88 100644
--- a/rsvg_internals/src/lib.rs
+++ b/rsvg_internals/src/lib.rs
@@ -48,6 +48,7 @@ pub use handle::{
     rsvg_handle_acquire_stream,
     rsvg_handle_defs_lookup,
     rsvg_handle_rust_cascade,
+    rsvg_handle_rust_close,
     rsvg_handle_rust_free,
     rsvg_handle_rust_get_base_gfile,
     rsvg_handle_rust_get_dpi_x,
@@ -64,20 +65,14 @@ pub use handle::{
     rsvg_handle_rust_set_flags,
     rsvg_handle_rust_set_load_state,
     rsvg_handle_rust_steal_result,
+    rsvg_handle_rust_write,
 };
 
-pub use load::{rsvg_load_close, rsvg_load_free, rsvg_load_new, rsvg_load_write};
-
 pub use node::rsvg_node_unref;
 
 pub use structure::rsvg_node_svg_get_size;
 
-pub use xml::{
-    rsvg_xml_state_error,
-    rsvg_xml_state_free,
-    rsvg_xml_state_new,
-    rsvg_xml_state_tree_is_valid,
-};
+pub use xml::rsvg_xml_state_error;
 
 #[macro_use]
 mod log;
diff --git a/rsvg_internals/src/load.rs b/rsvg_internals/src/load.rs
index cc62be0a..a4b0965e 100644
--- a/rsvg_internals/src/load.rs
+++ b/rsvg_internals/src/load.rs
@@ -1,12 +1,7 @@
 use gio;
-use glib::translate::*;
 use glib::{Bytes, Cast};
-use glib_sys;
 
-use std::slice;
-
-use error::set_gerror;
-use handle::LoadOptions;
+use handle::{LoadOptions, RsvgHandle};
 use xml::XmlState;
 use xml2_load::{xml_state_load_from_possibly_compressed_stream, ParseFromStreamError};
 
@@ -18,14 +13,14 @@ use xml2_load::{xml_state_load_from_possibly_compressed_stream, ParseFromStreamE
 //
 // This struct maintains the loading context while an RsvgHandle is being
 // populated with data, in case the caller is using write()/close().
-pub struct LoadContext<'a> {
+pub struct LoadContext {
     load_options: LoadOptions,
 
     state: LoadState,
 
     buffer: Vec<u8>,
 
-    xml: &'a mut XmlState,
+    xml: Option<XmlState>,
 }
 
 #[derive(Copy, Clone)]
@@ -35,13 +30,13 @@ enum LoadState {
     Closed,
 }
 
-impl<'a> LoadContext<'a> {
-    pub fn new(xml: &mut XmlState, load_options: LoadOptions) -> LoadContext {
+impl LoadContext {
+    pub fn new(handle: *mut RsvgHandle, load_options: LoadOptions) -> LoadContext {
         LoadContext {
             load_options,
             state: LoadState::Start,
             buffer: Vec::new(),
-            xml,
+            xml: Some(XmlState::new(handle)),
         }
     }
 
@@ -58,13 +53,13 @@ impl<'a> LoadContext<'a> {
         self.buffer.extend_from_slice(buf);
     }
 
-    pub fn close(&mut self) -> Result<(), ParseFromStreamError> {
+    pub fn close(&mut self) -> Result<XmlState, ParseFromStreamError> {
         let state = self.state;
 
         match state {
-            LoadState::Start | LoadState::Closed => {
+            LoadState::Start => {
                 self.state = LoadState::Closed;
-                Ok(())
+                Ok(self.xml.take().unwrap())
             }
 
             LoadState::Reading => {
@@ -74,86 +69,15 @@ impl<'a> LoadContext<'a> {
                 let stream = gio::MemoryInputStream::new_from_bytes(&bytes);
 
                 xml_state_load_from_possibly_compressed_stream(
-                    &mut self.xml,
+                    self.xml.as_mut().unwrap(),
                     &self.load_options,
                     stream.upcast(),
                     None,
                 )
-            }
-        }
-    }
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn rsvg_load_new<'a>(
-    raw_xml: *mut XmlState,
-    flags: u32,
-) -> *mut LoadContext<'a> {
-    assert!(!raw_xml.is_null());
-
-    let xml = &mut *raw_xml;
-    let load_options = LoadOptions::from_flags(flags);
-
-    Box::into_raw(Box::new(LoadContext::new(xml, load_options)))
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn rsvg_load_free(raw_load_ctx: *mut LoadContext) -> *mut XmlState {
-    assert!(!raw_load_ctx.is_null());
-
-    let load_ctx = &mut *raw_load_ctx;
-
-    let xml = load_ctx.xml as *mut _;
-
-    Box::from_raw(raw_load_ctx);
-
-    xml
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn rsvg_load_write(
-    raw_load_ctx: *mut LoadContext,
-    buf: *const u8,
-    size: usize,
-) {
-    assert!(!raw_load_ctx.is_null());
-
-    let load_ctx = &mut *raw_load_ctx;
-    let slice = slice::from_raw_parts(buf, size);
-
-    load_ctx.write(slice);
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn rsvg_load_close(
-    raw_load_ctx: *mut LoadContext,
-    error: *mut *mut glib_sys::GError,
-) -> glib_sys::gboolean {
-    assert!(!raw_load_ctx.is_null());
-
-    let load_ctx = &mut *raw_load_ctx;
-
-    match load_ctx.close() {
-        Ok(()) => true.to_glib(),
-
-        Err(e) => {
-            match e {
-                ParseFromStreamError::CouldNotCreateXmlParser => {
-                    set_gerror(error, 0, "Error creating XML parser");
-                }
-
-                ParseFromStreamError::IoError(e) => {
-                    if !error.is_null() {
-                        *error = e.to_glib_full() as *mut _;
-                    }
-                }
-
-                ParseFromStreamError::XmlParseError(s) => {
-                    set_gerror(error, 0, &s);
-                }
+                .and_then(|_| Ok(self.xml.take().unwrap()))
             }
 
-            false.to_glib()
+            LoadState::Closed => unreachable!(),
         }
     }
 }
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index b6a41a8f..ce0e2e22 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -13,7 +13,7 @@ use attributes::Attribute;
 use create_node::create_node_and_register_id;
 use css::{self, CssStyles};
 use defs::Defs;
-use error::{set_gerror, LoadingError};
+use error::LoadingError;
 use handle::{self, RsvgHandle};
 use node::{node_new, Node, NodeType};
 use property_bag::PropertyBag;
@@ -597,17 +597,6 @@ fn parse_xml_stylesheet_processing_instruction(data: &str) -> Result<Vec<(String
     unreachable!();
 }
 
-#[no_mangle]
-pub extern "C" fn rsvg_xml_state_new(handle: *mut RsvgHandle) -> *mut XmlState {
-    Box::into_raw(Box::new(XmlState::new(handle)))
-}
-
-#[no_mangle]
-pub unsafe extern "C" fn rsvg_xml_state_free(xml: *mut XmlState) {
-    assert!(!xml.is_null());
-    Box::from_raw(xml);
-}
-
 #[no_mangle]
 pub unsafe extern "C" fn rsvg_xml_state_error(xml: *mut XmlState, msg: *const libc::c_char) {
     assert!(!xml.is_null());
@@ -621,24 +610,6 @@ pub unsafe extern "C" fn rsvg_xml_state_error(xml: *mut XmlState, msg: *const li
     xml.error(&msg);
 }
 
-#[no_mangle]
-pub unsafe extern "C" fn rsvg_xml_state_tree_is_valid(
-    xml: *mut XmlState,
-    error: *mut *mut glib_sys::GError,
-) -> glib_sys::gboolean {
-    assert!(!xml.is_null());
-    let xml = &mut *xml;
-
-    match xml.validate_tree() {
-        Ok(()) => true.to_glib(),
-
-        Err(e) => {
-            set_gerror(error, 0, &format!("{}", e));
-            false.to_glib()
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;