From 21cff3bca2055587f7a6a85a8cc7691111d43c50 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Tue, 28 Oct 2014 17:10:41 -0700 Subject: Fix several cppcheck warnings. --- src/pugixml.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 13ed4b8..b39aad0 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -2520,8 +2520,6 @@ PUGI__NS_BEGIN PUGI__PUSHNODE(node_doctype); cursor->value = mark; - - PUGI__POPNODE(); } } else if (*s == 0 && endch == '-') PUGI__THROW_ERROR(status_bad_comment, s); @@ -3660,7 +3658,7 @@ PUGI__NS_BEGIN while (node != root); } - PUGI__FN bool has_declaration(const xml_node node) + PUGI__FN bool has_declaration(xml_node node) { for (xml_node child = node.first_child(); child; child = child.next_sibling()) { @@ -3691,7 +3689,7 @@ PUGI__NS_BEGIN return true; } - PUGI__FN bool allow_move(const xml_node parent, const xml_node child) + PUGI__FN bool allow_move(xml_node parent, xml_node child) { // check that child can be a child of parent if (!allow_insert_child(parent.type(), child.type())) @@ -7323,7 +7321,7 @@ PUGI__NS_BEGIN prefix_length = pos ? static_cast(pos - name) : 0; } - bool operator()(const xml_attribute a) const + bool operator()(xml_attribute a) const { const char_t* name = a.name(); @@ -7333,7 +7331,7 @@ PUGI__NS_BEGIN } }; - PUGI__FN const char_t* namespace_uri(const xml_node node) + PUGI__FN const char_t* namespace_uri(xml_node node) { namespace_uri_predicate pred = node.name(); @@ -7351,7 +7349,7 @@ PUGI__NS_BEGIN return PUGIXML_TEXT(""); } - PUGI__FN const char_t* namespace_uri(const xml_attribute attr, const xml_node parent) + PUGI__FN const char_t* namespace_uri(xml_attribute attr, xml_node parent) { namespace_uri_predicate pred = attr.name(); -- cgit v1.2.3 From 25926c6206a47ffb5cbfb3702e2df22038176205 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sat, 1 Nov 2014 10:44:56 +0100 Subject: XPath: Fix undefined behavior while calling memcpy Calling memcpy(x, 0, 0) is technically undefined (although it should usually be a no-op). Fixes #20. --- src/pugixml.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/pugixml.cpp b/src/pugixml.cpp index b39aad0..4b1d5ab 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -7764,6 +7764,8 @@ PUGI__NS_BEGIN void append(const xpath_node* begin_, const xpath_node* end_, xpath_allocator* alloc) { + if (begin_ == end_) return; + size_t size_ = static_cast(_end - _begin); size_t capacity = static_cast(_eos - _begin); size_t count = static_cast(end_ - begin_); -- cgit v1.2.3 From e9948b4b05ca23cb95a6ca75ce4ee840e1fbda9b Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sun, 2 Nov 2014 09:30:56 +0100 Subject: Fix undefined behavior while calling memcpy Calling memcpy(x, 0, 0) is technically undefined (although it should usually be a no-op). --- src/pugixml.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 4b1d5ab..59e8f79 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -1352,7 +1352,11 @@ PUGI__NS_BEGIN char_t* buffer = static_cast(xml_memory::allocate((length + 1) * sizeof(char_t))); if (!buffer) return false; - memcpy(buffer, contents, length * sizeof(char_t)); + if (contents) + memcpy(buffer, contents, length * sizeof(char_t)); + else + assert(length == 0); + buffer[length] = 0; out_buffer = buffer; -- cgit v1.2.3 From 3950ee0433aaed3c35eaee7548bae5b00ee22a80 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Mon, 3 Nov 2014 18:33:08 +0100 Subject: XPath: Refactor predicate application Split number/boolean filtering logic into two functions. This creates an extra copy of a remove_if-like algorithm, but moves the type check out of the loop and results in better organized filtering code. Consolidate test-based dispatch into apply_predicate (which is now a member function). --- src/pugixml.cpp | 85 +++++++++++++++++++++++++++++++++------------------------ 1 file changed, 50 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 59e8f79..c8de68a 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -8503,44 +8503,59 @@ PUGI__NS_BEGIN } } - static void apply_predicate(xpath_node_set_raw& ns, size_t first, xpath_ast_node* expr, const xpath_stack& stack, bool once) + static void apply_predicate_boolean(xpath_node_set_raw& ns, size_t first, xpath_ast_node* expr, const xpath_stack& stack, bool once) { assert(ns.size() >= first); + assert(expr->rettype() != xpath_type_number); size_t i = 1; size_t size = ns.size() - first; - + xpath_node* last = ns.begin() + first; // remove_if... or well, sort of for (xpath_node* it = last; it != ns.end(); ++it, ++i) { xpath_context c(*it, i, size); - - if (expr->rettype() == xpath_type_number) + + if (expr->eval_boolean(c, stack)) { - if (expr->eval_number(c, stack) == i) - { - *last++ = *it; + *last++ = *it; - if (once) break; - } + if (once) break; } - else + } + + ns.truncate(last); + } + + static void apply_predicate_number(xpath_node_set_raw& ns, size_t first, xpath_ast_node* expr, const xpath_stack& stack, bool once) + { + assert(ns.size() >= first); + assert(expr->rettype() == xpath_type_number); + + size_t i = 1; + size_t size = ns.size() - first; + + xpath_node* last = ns.begin() + first; + + // remove_if... or well, sort of + for (xpath_node* it = last; it != ns.end(); ++it, ++i) + { + xpath_context c(*it, i, size); + + if (expr->eval_number(c, stack) == i) { - if (expr->eval_boolean(c, stack)) - { - *last++ = *it; + *last++ = *it; - if (once) break; - } + if (once) break; } } - + ns.truncate(last); } - static void apply_predicate_const(xpath_node_set_raw& ns, size_t first, xpath_ast_node* expr, const xpath_stack& stack) + static void apply_predicate_number_const(xpath_node_set_raw& ns, size_t first, xpath_ast_node* expr, const xpath_stack& stack) { assert(ns.size() >= first); assert(expr->rettype() == xpath_type_number); @@ -8568,21 +8583,28 @@ PUGI__NS_BEGIN ns.truncate(last); } + void apply_predicate(xpath_node_set_raw& ns, size_t first, const xpath_stack& stack, bool once) + { + if (ns.size() == first) return; + + assert(_type == ast_filter || _type == ast_predicate); + + if (_test == predicate_constant || _test == predicate_constant_one) + apply_predicate_number_const(ns, first, _right, stack); + else if (_right->rettype() == xpath_type_number) + apply_predicate_number(ns, first, _right, stack, once); + else + apply_predicate_boolean(ns, first, _right, stack, once); + } + void apply_predicates(xpath_node_set_raw& ns, size_t first, const xpath_stack& stack, nodeset_eval_t eval) { if (ns.size() == first) return; - + bool last_once = eval_once(ns.type() == xpath_node_set::type_sorted, eval); for (xpath_ast_node* pred = _right; pred; pred = pred->_next) - { - assert(pred->_type == ast_predicate); - - if (pred->_test == predicate_constant || pred->_test == predicate_constant_one) - apply_predicate_const(ns, first, pred->_right, stack); - else - apply_predicate(ns, first, pred->_right, stack, !pred->_next && last_once); - } + pred->apply_predicate(ns, first, stack, !pred->_next && last_once); } bool step_push(xpath_node_set_raw& ns, xml_attribute_struct* a, xml_node_struct* parent, xpath_allocator* alloc) @@ -9660,16 +9682,9 @@ PUGI__NS_BEGIN // either expression is a number or it contains position() call; sort by document order if (_test != predicate_posinv) set.sort_do(); - if (_test == predicate_constant || _test == predicate_constant_one) - { - apply_predicate_const(set, 0, _right, stack); - } - else - { - bool once = eval_once(set.type() == xpath_node_set::type_sorted, eval); + bool once = eval_once(set.type() == xpath_node_set::type_sorted, eval); - apply_predicate(set, 0, _right, stack, once); - } + apply_predicate(set, 0, stack, once); return set; } -- cgit v1.2.3