[librsvg: 12/16] Use Xml2Parser for the user_data of libxml2 SAX callbacks instead of XmlState
- From: Federico Mena Quintero <federico src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [librsvg: 12/16] Use Xml2Parser for the user_data of libxml2 SAX callbacks instead of XmlState
- Date: Fri, 11 Oct 2019 23:43:25 +0000 (UTC)
commit ee6b6e1153e79b1fb61b4f611cac41f3a322651d
Author: Federico Mena Quintero <federico gnome org>
Date: Fri Oct 11 11:08:59 2019 -0500
Use Xml2Parser for the user_data of libxml2 SAX callbacks instead of XmlState
In a subsequent commit, we'll need the SAX callbacks can have access
to the xmlParserCtxtPtr; libxml2 doesn't pass it passed to them, so we
need to somehow put it in the user_data.
To do this, we make the callbacks use the Xml2Parser struct as their
user_data, as Xml2Parser.parser is the xmlParserCtxPtr. Doing this
involves a bunch of stuff:
* We also need the XmlState in the user_data, so we add a field for
the XmlState to the Xml2Parser struct. To do this we now operate in
terms of Rc<XmlState>.
* When XmlState wants to create another Xml2Parser context, when
handling xi:include elements, it needs to pass another reference to
itself to the Xml2Parser. So, now XmlState stores a weak reference
to itself.
* However, Xml2Parser uses functions from XmlState that take &mut self to
create new elements and such. This is awkward in face of the
Rc<XmlState>. So, we move all the mutable fields of XmlState to a
RefCell<XmlStateInner> struct. This lets the methods that were &mut
self now be able to selectively borrow() or borrow_mut() the inner
struct.
* Finally - Xml2Parser::from_stream() now returns a
Result<Box<Xml2Parser>>; it needs to be boxed since the
pointer-to-the-Xml2Parser passed as a user_data to libxml2 outlives
that function, so the Xml2Parser needs to be in the heap.
rsvg_internals/src/xml.rs | 152 +++++++++++++++++++++++-----------------
rsvg_internals/src/xml2_load.rs | 61 +++++++++-------
2 files changed, 126 insertions(+), 87 deletions(-)
---
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index 7a866145..1853d677 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -5,6 +5,8 @@ use libc;
use markup5ever::{local_name, LocalName};
use std::collections::HashMap;
use std::str;
+use std::rc::{Rc, Weak};
+use std::cell::RefCell;
use crate::allowed_url::AllowedUrl;
use crate::create_node::create_node_and_register_id;
@@ -64,7 +66,8 @@ extern "C" {
/// that context, all XML events will be forwarded to it, and processed in one of the `XmlHandler`
/// trait objects. Normally the context refers to a `NodeCreationContext` implementation which is
/// what creates normal graphical elements.
-pub struct XmlState {
+struct XmlStateInner {
+ weak: Option<Weak<XmlState>>,
tree_root: Option<RsvgNode>,
ids: Option<HashMap<String, RsvgNode>>,
css_rules: Option<CssRules>,
@@ -72,6 +75,10 @@ pub struct XmlState {
current_node: Option<RsvgNode>,
entities: HashMap<String, XmlEntityPtr>,
+}
+
+pub struct XmlState {
+ inner: RefCell<XmlStateInner>,
load_options: LoadOptions,
}
@@ -88,33 +95,38 @@ enum AcquireError {
FatalError,
}
+impl XmlStateInner {
+ fn context(&self) -> Context {
+ // We can unwrap since the stack is never empty
+ self.context_stack.last().unwrap().clone()
+ }
+}
+
impl XmlState {
fn new(load_options: &LoadOptions) -> XmlState {
XmlState {
- tree_root: None,
- ids: Some(HashMap::new()),
- css_rules: Some(CssRules::default()),
- context_stack: vec![Context::Start],
- current_node: None,
- entities: HashMap::new(),
+ inner: RefCell::new(XmlStateInner {
+ weak: None,
+ tree_root: None,
+ ids: Some(HashMap::new()),
+ css_rules: Some(CssRules::default()),
+ context_stack: vec![Context::Start],
+ current_node: None,
+ entities: HashMap::new(),
+ }),
+
load_options: load_options.clone(),
}
}
- fn set_root(&mut self, root: &RsvgNode) {
- if self.tree_root.is_some() {
- panic!("The tree root has already been set");
- }
-
- self.tree_root = Some(root.clone());
- }
+ fn steal_result(&self) -> Result<Svg, LoadingError> {
+ let mut inner = self.inner.borrow_mut();
- fn steal_result(&mut self) -> Result<Svg, LoadingError> {
- match self.tree_root {
+ match inner.tree_root {
None => Err(LoadingError::SvgHasNoElements),
Some(ref root) if root.borrow().get_type() == NodeType::Svg => {
- let root = self.tree_root.take().unwrap();
- let css_rules = self.css_rules.as_ref().unwrap();
+ let root = inner.tree_root.take().unwrap();
+ let css_rules = inner.css_rules.as_ref().unwrap();
for mut node in root.descendants() {
node.borrow_mut().set_style(css_rules);
@@ -122,7 +134,7 @@ impl XmlState {
Ok(Svg::new(
root,
- self.ids.take().unwrap(),
+ inner.ids.take().unwrap(),
self.load_options.clone(),
))
}
@@ -130,13 +142,8 @@ impl XmlState {
}
}
- fn context(&self) -> Context {
- // We can unwrap since the stack is never empty
- self.context_stack.last().unwrap().clone()
- }
-
- pub fn start_element(&mut self, name: &str, pbag: &PropertyBag) {
- let context = self.context();
+ pub fn start_element(&self, name: &str, pbag: &PropertyBag) {
+ let context = self.inner.borrow().context();
if let Context::FatalError = context {
return;
@@ -157,11 +164,11 @@ impl XmlState {
Context::FatalError => unreachable!(),
};
- self.context_stack.push(new_context);
+ self.inner.borrow_mut().context_stack.push(new_context);
}
- pub fn end_element(&mut self, _name: &str) {
- let context = self.context();
+ pub fn end_element(&self, _name: &str) {
+ let context = self.inner.borrow().context();
match context {
Context::Start => panic!("end_element: XML handler stack is empty!?"),
@@ -173,11 +180,11 @@ impl XmlState {
}
// We can unwrap since start_element() always adds a context to the stack
- self.context_stack.pop().unwrap();
+ self.inner.borrow_mut().context_stack.pop().unwrap();
}
- pub fn characters(&mut self, text: &str) {
- let context = self.context();
+ pub fn characters(&self, text: &str) {
+ let context = self.inner.borrow().context();
match context {
// This is character data before the first element, i.e. something like
@@ -195,7 +202,7 @@ impl XmlState {
}
}
- pub fn processing_instruction(&mut self, target: &str, data: &str) {
+ pub fn processing_instruction(&self, target: &str, data: &str) {
if target != "xml-stylesheet" {
return;
}
@@ -218,11 +225,13 @@ impl XmlState {
&& type_.as_ref().map(String::as_str) == Some("text/css")
&& href.is_some()
{
+ let mut inner = self.inner.borrow_mut();
+
if let Ok(aurl) =
AllowedUrl::from_href(&href.unwrap(), self.load_options.base_url.as_ref())
{
// FIXME: handle CSS errors
- let css_rules = self.css_rules.as_mut().unwrap();
+ let css_rules = inner.css_rules.as_mut().unwrap();
let _ = css_rules.load_css(&aurl);
} else {
self.error("disallowed URL in xml-stylesheet");
@@ -233,20 +242,22 @@ impl XmlState {
}
}
- pub fn error(&mut self, msg: &str) {
+ pub fn error(&self, msg: &str) {
// FIXME: aggregate the errors and expose them to the public result
rsvg_log!("XML error: {}", msg);
- self.context_stack.push(Context::FatalError);
+ self.inner.borrow_mut().context_stack.push(Context::FatalError);
}
pub fn entity_lookup(&self, entity_name: &str) -> Option<XmlEntityPtr> {
- self.entities.get(entity_name).map(|v| *v)
+ self.inner.borrow().entities.get(entity_name).map(|v| *v)
}
- pub fn entity_insert(&mut self, entity_name: &str, entity: XmlEntityPtr) {
- let old_value = self.entities.insert(entity_name.to_string(), entity);
+ pub fn entity_insert(&self, entity_name: &str, entity: XmlEntityPtr) {
+ let mut inner = self.inner.borrow_mut();
+
+ let old_value = inner.entities.insert(entity_name.to_string(), entity);
if let Some(v) = old_value {
unsafe {
@@ -255,48 +266,58 @@ impl XmlState {
}
}
- fn element_creation_start_element(&mut self, name: &str, pbag: &PropertyBag) -> Context {
+ fn element_creation_start_element(&self, name: &str, pbag: &PropertyBag) -> Context {
match name {
"include" => self.xinclude_start_element(name, pbag),
_ => {
- let ids = self.ids.as_mut().unwrap();
+ let mut inner = self.inner.borrow_mut();
+
+ let ids = inner.ids.as_mut().unwrap();
let mut node = create_node_and_register_id(name, pbag, ids);
- let parent = self.current_node.clone();
+ let parent = inner.current_node.clone();
node.borrow_mut()
.set_atts(parent.as_ref(), pbag, self.load_options.locale());
if let Some(mut parent) = parent {
parent.append(node.clone());
} else {
- self.set_root(&node);
+ if inner.tree_root.is_some() {
+ panic!("The tree root has already been set");
+ }
+
+ inner.tree_root = Some(node.clone());
}
- self.current_node = Some(node);
+ inner.current_node = Some(node);
Context::ElementCreation
}
}
}
- fn element_creation_end_element(&mut self) {
- let node = self.current_node.take().unwrap();
+ fn element_creation_end_element(&self) {
+ let mut inner = self.inner.borrow_mut();
+
+ let node = inner.current_node.take().unwrap();
if node.borrow().get_type() == NodeType::Style {
- let css_rules = self.css_rules.as_mut().unwrap();
+ let css_rules = inner.css_rules.as_mut().unwrap();
let css_data = node.borrow().get_impl::<NodeStyle>().get_css(&node);
css_rules.parse(self.load_options.base_url.as_ref(), &css_data);
}
- self.current_node = node.parent();
+ inner.current_node = node.parent();
}
fn element_creation_characters(&self, text: &str) {
+ let inner = self.inner.borrow();
+
if text.len() != 0 {
// When the last child is a Chars node we can coalesce
// the text and avoid screwing up the Pango layouts
- let chars_node = if let Some(child) = self
+ let chars_node = if let Some(child) = inner
.current_node
.as_ref()
.unwrap()
@@ -313,7 +334,7 @@ impl XmlState {
Box::new(NodeChars::new()),
));
- let mut node = self.current_node.as_ref().unwrap().clone();
+ let mut node = inner.current_node.as_ref().unwrap().clone();
node.append(child.clone());
child
@@ -323,7 +344,7 @@ impl XmlState {
}
}
- fn xinclude_start_element(&mut self, _name: &str, pbag: &PropertyBag) -> Context {
+ fn xinclude_start_element(&self, _name: &str, pbag: &PropertyBag) -> Context {
let mut href = None;
let mut parse = None;
let mut encoding = None;
@@ -364,7 +385,7 @@ impl XmlState {
}
fn xinclude_fallback_start_element(
- &mut self,
+ &self,
ctx: &XIncludeContext,
name: &str,
pbag: &PropertyBag,
@@ -381,8 +402,8 @@ impl XmlState {
}
}
- fn xinclude_fallback_characters(&mut self, ctx: &XIncludeContext, text: &str) {
- if ctx.need_fallback && self.current_node.is_some() {
+ fn xinclude_fallback_characters(&self, ctx: &XIncludeContext, text: &str) {
+ if ctx.need_fallback && self.inner.borrow().current_node.is_some() {
// We test for is_some() because with a bad "SVG" file like this:
//
// <xi:include href="blah"><xi:fallback>foo</xi:fallback></xi:include>
@@ -394,7 +415,7 @@ impl XmlState {
}
fn acquire(
- &mut self,
+ &self,
href: Option<&str>,
parse: Option<&str>,
encoding: Option<&str>,
@@ -431,7 +452,7 @@ impl XmlState {
}
fn acquire_text(
- &mut self,
+ &self,
aurl: &AllowedUrl,
encoding: Option<&str>,
) -> Result<(), AcquireError> {
@@ -463,7 +484,7 @@ impl XmlState {
Ok(())
}
- fn acquire_xml(&mut self, aurl: &AllowedUrl) -> Result<(), AcquireError> {
+ fn acquire_xml(&self, aurl: &AllowedUrl) -> Result<(), AcquireError> {
// FIXME: distinguish between "file not found" and "invalid XML"
let stream = io::acquire_stream(aurl, None).map_err(|e| match e {
@@ -484,11 +505,12 @@ impl XmlState {
// This can be called "in the middle" of an XmlState's processing status,
// for example, when including another XML file via xi:include.
fn parse_from_stream(
- &mut self,
+ &self,
stream: &gio::InputStream,
cancellable: Option<&gio::Cancellable>,
) -> Result<(), ParseFromStreamError> {
- Xml2Parser::from_stream(self, self.load_options.unlimited_size, stream, cancellable)
+ let strong = self.inner.borrow().weak.as_ref().unwrap().upgrade().unwrap();
+ Xml2Parser::from_stream(strong, self.load_options.unlimited_size, stream, cancellable)
.and_then(|parser| parser.parse())
}
@@ -500,7 +522,9 @@ impl XmlState {
impl Drop for XmlState {
fn drop(&mut self) {
unsafe {
- for (_key, entity) in self.entities.drain() {
+ let mut inner = self.inner.borrow_mut();
+
+ for (_key, entity) in inner.entities.drain() {
xmlFreeNode(entity);
}
}
@@ -553,14 +577,16 @@ pub fn xml_load_from_possibly_compressed_stream(
stream: &gio::InputStream,
cancellable: Option<&gio::Cancellable>,
) -> Result<Svg, LoadingError> {
- let mut xml = XmlState::new(load_options);
+ let state = Rc::new(XmlState::new(load_options));
+
+ state.inner.borrow_mut().weak = Some(Rc::downgrade(&state));
let stream = get_input_stream_for_loading(stream, cancellable)
.map_err(|e| ParseFromStreamError::IoError(e))?;
- xml.parse_from_stream(&stream, cancellable)?;
+ state.parse_from_stream(&stream, cancellable)?;
- xml.steal_result()
+ state.steal_result()
}
diff --git a/rsvg_internals/src/xml2_load.rs b/rsvg_internals/src/xml2_load.rs
index a40834bf..bb3b98d7 100644
--- a/rsvg_internals/src/xml2_load.rs
+++ b/rsvg_internals/src/xml2_load.rs
@@ -4,7 +4,7 @@
use gio;
use gio::prelude::*;
use std::borrow::Cow;
-use std::cell::RefCell;
+use std::cell::{Cell, RefCell};
use std::mem;
use std::ptr;
use std::rc::Rc;
@@ -39,7 +39,7 @@ fn get_xml2_sax_handler() -> xmlSAXHandler {
}
unsafe extern "C" fn rsvg_sax_serror_cb(user_data: *mut libc::c_void, error: xmlErrorPtr) {
- let state = (user_data as *mut XmlState).as_mut().unwrap();
+ let xml2_parser = &*(user_data as *mut Xml2Parser);
let error = error.as_ref().unwrap();
let level_name = match error.level {
@@ -66,7 +66,7 @@ unsafe extern "C" fn rsvg_sax_serror_cb(user_data: *mut libc::c_void, error: xml
column,
cstr(error.message)
);
- state.error(&full_error_message);
+ xml2_parser.state.error(&full_error_message);
}
fn free_xml_parser_and_doc(parser: xmlParserCtxtPtr) {
@@ -90,12 +90,12 @@ unsafe extern "C" fn sax_get_entity_cb(
user_data: *mut libc::c_void,
name: *const libc::c_char,
) -> xmlEntityPtr {
- let xml = &*(user_data as *mut XmlState);
+ let xml2_parser = &*(user_data as *mut Xml2Parser);
assert!(!name.is_null());
let name = utf8_cstr(name);
- xml.entity_lookup(name).unwrap_or(ptr::null_mut())
+ xml2_parser.state.entity_lookup(name).unwrap_or(ptr::null_mut())
}
unsafe extern "C" fn sax_entity_decl_cb(
@@ -106,7 +106,7 @@ unsafe extern "C" fn sax_entity_decl_cb(
_system_id: *const libc::c_char,
content: *const libc::c_char,
) {
- let xml = &mut *(user_data as *mut XmlState);
+ let xml2_parser = &*(user_data as *mut Xml2Parser);
assert!(!name.is_null());
@@ -128,7 +128,7 @@ unsafe extern "C" fn sax_entity_decl_cb(
assert!(!entity.is_null());
let name = utf8_cstr(name);
- xml.entity_insert(name, entity);
+ xml2_parser.state.entity_insert(name, entity);
}
unsafe extern "C" fn sax_unparsed_entity_decl_cb(
@@ -153,23 +153,23 @@ unsafe extern "C" fn sax_start_element_cb(
name: *const libc::c_char,
atts: *const *const libc::c_char,
) {
- let xml = &mut *(user_data as *mut XmlState);
+ let xml2_parser = &*(user_data as *mut Xml2Parser);
assert!(!name.is_null());
let name = utf8_cstr(name);
let pbag = PropertyBag::new_from_key_value_pairs(atts);
- xml.start_element(name, &pbag);
+ xml2_parser.state.start_element(name, &pbag);
}
unsafe extern "C" fn sax_end_element_cb(user_data: *mut libc::c_void, name: *const libc::c_char) {
- let xml = &mut *(user_data as *mut XmlState);
+ let xml2_parser = &*(user_data as *mut Xml2Parser);
assert!(!name.is_null());
let name = utf8_cstr(name);
- xml.end_element(name);
+ xml2_parser.state.end_element(name);
}
unsafe extern "C" fn sax_characters_cb(
@@ -177,7 +177,7 @@ unsafe extern "C" fn sax_characters_cb(
unterminated_text: *const libc::c_char,
len: libc::c_int,
) {
- let xml = &mut *(user_data as *mut XmlState);
+ let xml2_parser = &*(user_data as *mut Xml2Parser);
assert!(!unterminated_text.is_null());
assert!(len >= 0);
@@ -187,7 +187,7 @@ unsafe extern "C" fn sax_characters_cb(
let bytes = std::slice::from_raw_parts(unterminated_text as *const u8, len as usize);
let utf8 = str::from_utf8_unchecked(bytes);
- xml.characters(utf8);
+ xml2_parser.state.characters(utf8);
}
unsafe extern "C" fn sax_processing_instruction_cb(
@@ -195,7 +195,7 @@ unsafe extern "C" fn sax_processing_instruction_cb(
target: *const libc::c_char,
data: *const libc::c_char,
) {
- let xml = &mut *(user_data as *mut XmlState);
+ let xml2_parser = &*(user_data as *mut Xml2Parser);
assert!(!target.is_null());
let target = utf8_cstr(target);
@@ -206,7 +206,7 @@ unsafe extern "C" fn sax_processing_instruction_cb(
utf8_cstr(data)
};
- xml.processing_instruction(target, data);
+ xml2_parser.state.processing_instruction(target, data);
}
unsafe extern "C" fn sax_get_parameter_entity_cb(
@@ -314,17 +314,18 @@ fn init_libxml2() {
}
pub struct Xml2Parser {
- parser: xmlParserCtxtPtr,
+ parser: Cell<xmlParserCtxtPtr>,
+ state: Rc<XmlState>,
gio_error: Rc<RefCell<Option<glib::Error>>>,
}
impl Xml2Parser {
pub fn from_stream(
- xml: &mut XmlState,
+ state: Rc<XmlState>,
unlimited_size: bool,
stream: &gio::InputStream,
cancellable: Option<&gio::Cancellable>,
- ) -> Result<Xml2Parser, ParseFromStreamError> {
+ ) -> Result<Box<Xml2Parser>, ParseFromStreamError> {
init_libxml2();
// The Xml2Parser we end up creating, if
@@ -344,10 +345,16 @@ impl Xml2Parser {
let mut sax_handler = get_xml2_sax_handler();
+ let mut xml2_parser = Box::new(Xml2Parser {
+ parser: Cell::new(ptr::null_mut()),
+ state,
+ gio_error,
+ });
+
unsafe {
let parser = xmlCreateIOParserCtxt(
&mut sax_handler,
- xml as *mut _ as *mut _,
+ xml2_parser.as_mut() as *mut _ as *mut _,
Some(stream_ctx_read),
Some(stream_ctx_close),
Box::into_raw(ctx) as *mut _,
@@ -359,15 +366,20 @@ impl Xml2Parser {
// stream_ctx_close function
Err(ParseFromStreamError::CouldNotCreateXmlParser)
} else {
+ xml2_parser.parser.set(parser);
+
set_xml_parse_options(parser, unlimited_size);
- Ok(Xml2Parser { parser, gio_error })
+
+ Ok(xml2_parser)
}
}
}
pub fn parse(&self) -> Result<(), ParseFromStreamError> {
unsafe {
- let xml_parse_success = xmlParseDocument(self.parser) == 0;
+ let parser = self.parser.get();
+
+ let xml_parse_success = xmlParseDocument(parser) == 0;
let mut err_ref = self.gio_error.borrow_mut();
@@ -376,7 +388,7 @@ impl Xml2Parser {
if let Some(io_error) = io_error {
Err(ParseFromStreamError::IoError(io_error))
} else if !xml_parse_success {
- let xerr = xmlCtxtGetLastError(self.parser as *mut _);
+ let xerr = xmlCtxtGetLastError(parser as *mut _);
let msg = xml2_error_to_string(xerr);
Err(ParseFromStreamError::XmlParseError(msg))
} else {
@@ -388,8 +400,9 @@ impl Xml2Parser {
impl Drop for Xml2Parser {
fn drop(&mut self) {
- free_xml_parser_and_doc(self.parser);
- self.parser = ptr::null_mut();
+ let parser = self.parser.get();
+ free_xml_parser_and_doc(parser);
+ self.parser.set(ptr::null_mut());
}
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
