From 814443b147e6159a0ad2842fabc1288ec6a0ee24 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sat, 11 Apr 2015 22:37:38 -0700 Subject: Fix exception type for out-of-memory for XPath variables When parsing XPath variables, we need to perform a heap allocation; if it fails, an xpath_exception instead of bad_alloc used to be thrown. Now we throw the exception of a correct type so that xpath_exception means 'parsing error'. --- src/pugixml.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'src/pugixml.cpp') diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 5b77a27..5190937 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -7715,7 +7715,7 @@ PUGI__NS_BEGIN } } - PUGI__FN xpath_variable* get_variable_scratch(char_t (&buffer)[32], xpath_variable_set* set, const char_t* begin, const char_t* end) + PUGI__FN bool get_variable_scratch(char_t (&buffer)[32], xpath_variable_set* set, const char_t* begin, const char_t* end, xpath_variable** out_result) { size_t length = static_cast(end - begin); char_t* scratch = buffer; @@ -7724,19 +7724,19 @@ PUGI__NS_BEGIN { // need to make dummy on-heap copy scratch = static_cast(xml_memory::allocate((length + 1) * sizeof(char_t))); - if (!scratch) return 0; + if (!scratch) return false; } // copy string to zero-terminated buffer and perform lookup memcpy(scratch, begin, length * sizeof(char_t)); scratch[length] = 0; - xpath_variable* result = set->get(scratch); + *out_result = set->get(scratch); // free dummy buffer if (scratch != buffer) xml_memory::deallocate(scratch); - return result; + return true; } PUGI__NS_END @@ -10309,7 +10309,9 @@ PUGI__NS_BEGIN if (!_variables) throw_error("Unknown variable: variable set is not provided"); - xpath_variable* var = get_variable_scratch(_scratch, _variables, name.begin, name.end); + xpath_variable* var = 0; + if (!get_variable_scratch(_scratch, _variables, name.begin, name.end, &var)) + throw_error_oom(); if (!var) throw_error("Unknown variable: variable set does not contain the given name"); -- cgit v1.2.3 From 99afee183225f6f33f3289030c992738ccaf13fe Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sun, 12 Apr 2015 01:32:25 -0700 Subject: Move zero-termination out of as_utf8_end as_utf8_end was used with std::string, where writing an extra zero-terminating character should *probably* always work (at least if size is positive) but is not ideal. The only place that needed to zero-terminate was convert_path_heap. --- src/pugixml.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/pugixml.cpp') diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 5190937..6b3e87e 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -1691,9 +1691,6 @@ PUGI__NS_BEGIN assert(begin + size == end); (void)!end; - - // zero-terminate - buffer[size] = 0; } #ifndef PUGIXML_NO_STL @@ -4295,6 +4292,9 @@ PUGI__NS_BEGIN // second pass: convert to utf8 as_utf8_end(result, size, str, length); + // zero-terminate + result[size] = 0; + return result; } -- cgit v1.2.3 From d6f7766172bd3dcd6b286888f5bdfdcb1953f3ba Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sun, 12 Apr 2015 02:05:59 -0700 Subject: Optimize xml_node::path() to use 1 allocation Instead of reallocating the string for every tree level just do two passes over the ancestor chain. --- src/pugixml.cpp | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) (limited to 'src/pugixml.cpp') diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 6b3e87e..619cc7b 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -4056,6 +4056,7 @@ PUGI__NS_BEGIN return status_ok; } + // This function assumes that buffer has extra sizeof(char_t) writable bytes after size PUGI__FN size_t zero_terminate_buffer(void* buffer, size_t size, xml_encoding encoding) { // We only need to zero-terminate if encoding conversion does not do it for us @@ -5328,20 +5329,35 @@ namespace pugi #ifndef PUGIXML_NO_STL PUGI__FN string_t xml_node::path(char_t delimiter) const { - xml_node cursor = *this; // Make a copy. - - string_t result = cursor.name(); + if (!_root) return string_t(); + + size_t offset = 0; - while (cursor.parent()) + for (xml_node_struct* i = _root; i; i = i->parent) { - cursor = cursor.parent(); - - string_t temp = cursor.name(); - temp += delimiter; - temp += result; - result.swap(temp); + offset += (i != _root); + offset += i->name ? impl::strlength(i->name) : 0; + } + + string_t result; + result.resize(offset); + + for (xml_node_struct* j = _root; j; j = j->parent) + { + if (j != _root) + result[--offset] = delimiter; + + if (j->name && *j->name) + { + size_t length = impl::strlength(j->name); + + offset -= length; + memcpy(&result[offset], j->name, length * sizeof(char_t)); + } } + assert(offset == 0); + return result; } #endif @@ -6188,12 +6204,14 @@ namespace pugi PUGI__FN bool xml_document::save_file(const char* path_, const char_t* indent, unsigned int flags, xml_encoding encoding) const { FILE* file = fopen(path_, (flags & format_save_file_text) ? "w" : "wb"); + return impl::save_file_impl(*this, file, indent, flags, encoding); } PUGI__FN bool xml_document::save_file(const wchar_t* path_, const char_t* indent, unsigned int flags, xml_encoding encoding) const { FILE* file = impl::open_file_wide(path_, (flags & format_save_file_text) ? L"w" : L"wb"); + return impl::save_file_impl(*this, file, indent, flags, encoding); } -- cgit v1.2.3 From a0d065cd22d1d43c417f6d3db88a04bf57b67ed0 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sun, 12 Apr 2015 03:03:56 -0700 Subject: Implment copyless copy for attributes Previously attributes that were copied with their node used string sharing, but standalone attributes that were copied using xml_node::*_copy(xml_attribute) were not. --- src/pugixml.cpp | 57 ++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 15 deletions(-) (limited to 'src/pugixml.cpp') diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 619cc7b..65854e7 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -557,11 +557,11 @@ PUGI__NS_BEGIN xml_extra_buffer* extra_buffers; }; - inline xml_allocator& get_allocator(const xml_node_struct* node) + template inline xml_allocator& get_allocator(const Object* object) { - assert(node); + assert(object); - return *reinterpret_cast(node->header & xml_memory_page_pointer_mask)->allocator; + return *reinterpret_cast(object->header & xml_memory_page_pointer_mask)->allocator; } template inline xml_document_struct& get_document(const Object* object) @@ -3824,6 +3824,15 @@ PUGI__NS_BEGIN } } + PUGI__FN void node_copy_attribute(xml_attribute_struct* da, xml_attribute_struct* sa) + { + xml_allocator& alloc = get_allocator(da); + xml_allocator* shared_alloc = (&alloc == &get_allocator(sa)) ? &alloc : 0; + + node_copy_string(da->name, da->header, xml_memory_page_name_allocated_mask, sa->name, sa->header, shared_alloc); + node_copy_string(da->value, da->header, xml_memory_page_value_allocated_mask, sa->value, sa->header, shared_alloc); + } + inline bool is_text_node(xml_node_struct* node) { xml_node_type type = PUGI__NODETYPE(node); @@ -4986,41 +4995,59 @@ namespace pugi PUGI__FN xml_attribute xml_node::append_copy(const xml_attribute& proto) { if (!proto) return xml_attribute(); + if (!impl::allow_insert_attribute(type())) return xml_attribute(); - xml_attribute result = append_attribute(proto.name()); - result.set_value(proto.value()); + xml_attribute a(impl::allocate_attribute(impl::get_allocator(_root))); + if (!a) return xml_attribute(); - return result; + impl::append_attribute(a._attr, _root); + impl::node_copy_attribute(a._attr, proto._attr); + + return a; } PUGI__FN xml_attribute xml_node::prepend_copy(const xml_attribute& proto) { if (!proto) return xml_attribute(); + if (!impl::allow_insert_attribute(type())) return xml_attribute(); - xml_attribute result = prepend_attribute(proto.name()); - result.set_value(proto.value()); + xml_attribute a(impl::allocate_attribute(impl::get_allocator(_root))); + if (!a) return xml_attribute(); - return result; + impl::prepend_attribute(a._attr, _root); + impl::node_copy_attribute(a._attr, proto._attr); + + return a; } PUGI__FN xml_attribute xml_node::insert_copy_after(const xml_attribute& proto, const xml_attribute& attr) { if (!proto) return xml_attribute(); + if (!impl::allow_insert_attribute(type())) return xml_attribute(); + if (!attr || !impl::is_attribute_of(attr._attr, _root)) return xml_attribute(); - xml_attribute result = insert_attribute_after(proto.name(), attr); - result.set_value(proto.value()); + xml_attribute a(impl::allocate_attribute(impl::get_allocator(_root))); + if (!a) return xml_attribute(); - return result; + impl::insert_attribute_after(a._attr, attr._attr, _root); + impl::node_copy_attribute(a._attr, proto._attr); + + return a; } PUGI__FN xml_attribute xml_node::insert_copy_before(const xml_attribute& proto, const xml_attribute& attr) { if (!proto) return xml_attribute(); + if (!impl::allow_insert_attribute(type())) return xml_attribute(); + if (!attr || !impl::is_attribute_of(attr._attr, _root)) return xml_attribute(); + + xml_attribute a(impl::allocate_attribute(impl::get_allocator(_root))); + if (!a) return xml_attribute(); - xml_attribute result = insert_attribute_before(proto.name(), attr); - result.set_value(proto.value()); + impl::insert_attribute_before(a._attr, attr._attr, _root); + impl::node_copy_attribute(a._attr, proto._attr); - return result; + return a; } PUGI__FN xml_node xml_node::append_child(xml_node_type type_) -- cgit v1.2.3