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(-) 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 97893ad738dfe3308d7e9793c301f24e6c9dc3a4 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sat, 1 Nov 2014 08:59:49 +0100 Subject: Fix first-time make config=coverage test Not sure why xargs -r is not the default... --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index d351e0f..457dd23 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ all: $(EXECUTABLE) ifeq ($(config),coverage) test: $(EXECUTABLE) - @find $(BUILD) -name '*.gcda' | xargs rm + @find $(BUILD) -name '*.gcda' | xargs -r rm ./$(EXECUTABLE) @gcov -b -c $(BUILD)/src/pugixml.cpp.gcda | sed -e '/./{H;$!d;}' -e 'x;/pugixml.cpp/!d;' @ls *.gcov | grep -v pugixml.cpp.gcov | xargs rm @@ -51,4 +51,4 @@ $(BUILD)/%.o: % -include $(OBJECTS:.o=.d) -.PHONY: all test clean \ No newline at end of file +.PHONY: all test clean -- 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(+) 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 f68a320a0203279db2e8692cc9e21f71551454e4 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Sat, 1 Nov 2014 11:45:13 +0100 Subject: tests: Improve test coverage --- tests/test_document.cpp | 4 ++++ tests/test_xpath_paths.cpp | 2 ++ 2 files changed, 6 insertions(+) diff --git a/tests/test_document.cpp b/tests/test_document.cpp index 2cc39a6..2774a07 100644 --- a/tests/test_document.cpp +++ b/tests/test_document.cpp @@ -301,6 +301,10 @@ TEST(document_load_file_error) CHECK(doc.load_file("filedoesnotexist").status == status_file_not_found); +#ifndef _WIN32 + CHECK(doc.load_file("/dev/tty").status == status_io_error); +#endif + test_runner::_memory_fail_threshold = 1; CHECK(doc.load_file("tests/data/small.xml").status == status_out_of_memory); } diff --git a/tests/test_xpath_paths.cpp b/tests/test_xpath_paths.cpp index df0dfa4..e51a395 100644 --- a/tests/test_xpath_paths.cpp +++ b/tests/test_xpath_paths.cpp @@ -664,6 +664,8 @@ TEST_XML(xpath_paths_optimize_step_once, "

") -- 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 +++++- tests/test_dom_modify.cpp | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) 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; diff --git a/tests/test_dom_modify.cpp b/tests/test_dom_modify.cpp index 07fe6dc..45cf3ea 100644 --- a/tests/test_dom_modify.cpp +++ b/tests/test_dom_modify.cpp @@ -1091,6 +1091,19 @@ TEST_XML(dom_node_append_buffer_fragment, "") CHECK_NODE(doc, STR("1234")); } +TEST_XML(dom_node_append_buffer_empty, "") +{ + xml_node node = doc.child(STR("node")); + + CHECK(node.append_buffer("", 0).status == status_no_document_element); + CHECK(node.append_buffer("", 0, parse_fragment).status == status_ok); + + CHECK(node.append_buffer(0, 0).status == status_no_document_element); + CHECK(node.append_buffer(0, 0, parse_fragment).status == status_ok); + + CHECK_NODE(doc, STR("")); +} + TEST_XML(dom_node_prepend_move, "foo") { xml_node child = doc.child(STR("node")).child(STR("child")); -- cgit v1.2.3 From 97a515f8733d9555278b6eb3f3dc244a9580c308 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Mon, 3 Nov 2014 07:50:00 +0100 Subject: tests: Add more XPath mod tests --- tests/test_xpath_operators.cpp | 48 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/test_xpath_operators.cpp b/tests/test_xpath_operators.cpp index c028224..57e3755 100644 --- a/tests/test_xpath_operators.cpp +++ b/tests/test_xpath_operators.cpp @@ -481,4 +481,52 @@ TEST(xpath_operators_associativity_arithmetic) CHECK_XPATH_NUMBER(c, STR("1-1+1"), 1); } +TEST(xpath_operators_mod) +{ + // Check that mod operator conforms to Java spec (since this is the only concrete source of information about XPath mod) + xml_node c; + + // Basic tests from spec + CHECK_XPATH_NUMBER(c, STR("5 mod 3"), 2); + CHECK_XPATH_NUMBER(c, STR("5 mod -3"), 2); + CHECK_XPATH_NUMBER(c, STR("-5 mod 3"), -2); + CHECK_XPATH_NUMBER(c, STR("-5 mod -3"), -2); + + // If either operand is NaN, the result is NaN + CHECK_XPATH_NUMBER_NAN(c, STR("(0 div 0) mod 3")); + CHECK_XPATH_NUMBER_NAN(c, STR("3 mod (0 div 0)")); + CHECK_XPATH_NUMBER_NAN(c, STR("(0 div 0) mod (0 div 0)")); + + // If the dividend is an infinity, or the divisor is a zero, or both, the result is NaN + CHECK_XPATH_NUMBER_NAN(c, STR("(1 div 0) mod 3")); + CHECK_XPATH_NUMBER_NAN(c, STR("(1 div 0) mod -3")); + CHECK_XPATH_NUMBER_NAN(c, STR("(-1 div 0) mod 3")); + CHECK_XPATH_NUMBER_NAN(c, STR("1 mod 0")); + CHECK_XPATH_NUMBER_NAN(c, STR("-1 mod 0")); + CHECK_XPATH_NUMBER_NAN(c, STR("(1 div 0) mod 0")); + CHECK_XPATH_NUMBER_NAN(c, STR("(-1 div 0) mod 0")); + + // If the dividend is finite and the divisor is an infinity, the result equals the dividend + CHECK_XPATH_NUMBER(c, STR("1 mod (1 div 0)"), 1); + CHECK_XPATH_NUMBER(c, STR("1 mod (-1 div 0)"), 1); + CHECK_XPATH_NUMBER(c, STR("-1 mod (1 div 0)"), -1); + CHECK_XPATH_NUMBER(c, STR("0 mod (1 div 0)"), 0); + CHECK_XPATH_NUMBER(c, STR("0 mod (-1 div 0)"), 0); + CHECK_XPATH_NUMBER(c, STR("100000 mod (1 div 0)"), 100000); + + // If the dividend is a zero and the divisor is finite, the result equals the dividend. + CHECK_XPATH_NUMBER(c, STR("0 mod 1000000"), 0); + CHECK_XPATH_NUMBER(c, STR("0 mod -1000000"), 0); + + // In the remaining cases ... the floating-point remainder r from the division of a dividend n by a divisor d + // is defined by the mathematical relation r = n - (d * q) where q is an integer that is negative only if n/d is + // negative and positive only if n/d is positive, and whose magnitude is as large as possible without exceeding the magnitude of the true + // mathematical quotient of n and d. + CHECK_XPATH_NUMBER(c, STR("9007199254740991 mod 2"), 1); + CHECK_XPATH_NUMBER(c, STR("9007199254740991 mod 3"), 1); + CHECK_XPATH_NUMBER(c, STR("18446744073709551615 mod 2"), 0); + CHECK_XPATH_NUMBER(c, STR("18446744073709551615 mod 3"), 1); + CHECK_XPATH_NUMBER(c, STR("115792089237316195423570985008687907853269984665640564039457584007913129639935 mod 2"), 0); + CHECK_XPATH_NUMBER(c, STR("115792089237316195423570985008687907853269984665640564039457584007913129639935 mod 3"), 1); +} #endif -- 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(-) 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