From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x92b.google.com (mail-ua1-x92b.google.com [IPv6:2607:f8b0:4864:20::92b]) by sourceware.org (Postfix) with ESMTPS id D9C9C3857C6F for ; Wed, 31 Mar 2021 16:59:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D9C9C3857C6F Received: by mail-ua1-x92b.google.com with SMTP id v23so6331906uaq.13 for ; Wed, 31 Mar 2021 09:59:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8K0N305vmrNfcCDKB4Rl7HX3iFrIwPAdJqgsxYDMbIM=; b=K/CyJ0JYUUS5iKdyxRbroMUPvX0Hs4Ead4xce9CI7RPvaCfQiCFoozHocKD+5/Pz8F qjLb69t+VbaTrCM6QTUh/onGEvTsnKW0ATLNQeUWvm592Jj4HfcoPZFVH5D8zhX7PoPa lbeyJLoXhobmUTskFmuP33WKonERLZoQMrSl/yXgpsjY9InLAzYQ8VIMuspEqu9gRYVE yoH5wzgnbsk067JK5LufEOOk1tMt7orgPkzPNDkDk5wGmU7i7EMloYPkNQBIBAEnEBsm vyqMeTf2VgzxqVW6kWiVf+68O0ysA13yrmNinQW3t9Cy6KDCG5PejlpeLj3PJZ6ogSm9 trkw== X-Gm-Message-State: AOAM531/8uEo1I+SdgQuhkfn5Yq6k0Ni88FvcP5VSPyAdbq7hbuVlB1K JHetD6tW2EKROwfLitNxVhit5zPmHwgXn/oTw7AvwrnVRmu1TA== X-Google-Smtp-Source: ABdhPJxL7roqSsojtYzSTALmU1A59qTMV8f1FYF6PokDxLC4y2k37gZ3Q7QNUJOiU26uwGhwFXPmeMVYRQky8b0gZ+w= X-Received: by 2002:ab0:1c45:: with SMTP id o5mr2367390uaj.13.1617209958065; Wed, 31 Mar 2021 09:59:18 -0700 (PDT) MIME-Version: 1.0 References: <20210331092352.619148-1-gprocida@google.com> In-Reply-To: From: Giuliano Procida Date: Wed, 31 Mar 2021 17:58:24 +0100 Message-ID: Subject: Re: [PATCH] XML reader: improve XML node traversal To: Matthias Maennich Cc: Giuliano Procida via Libabigail , Dodji Seketeli , kernel-team@android.com X-Spam-Status: No, score=-28.3 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SCC_5_SHORT_WORD_LINES, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Mar 2021 16:59:25 -0000 Thanks for the review. Each of the long lines was a non-standard iteration over nodes and actually deserved better. On Wed, 31 Mar 2021 at 15:24, Matthias Maennich wrote: > On Wed, Mar 31, 2021 at 10:23:51AM +0100, Giuliano Procida wrote: > >Bug 27616: libabigail treats empty and nearly empty XML elements > inconsistently > > > >The XML reader uses libxml's Reader API for parsing XML and traverses > >high-level XML nodes using reader cursor advancement. Low-level nodes > >are read into complete node trees and are traversed following > >children, next and (in one instance) parent pointers. Some > >intermediate-level nodes can be parsed both ways. > > > >A lot of code cares about node types, typically skipping over > >non-element nodes. Unfortunately, in a few places, the tracked "corpus > >node" needs to land on text (whitespace) within elements for the XML > >reader to work; stripping text nodes out causes some elements to be > >skipped. Almost all node tree traversals are very straightforward and > >can be expressed using simpler primitves than at present. > > > >This commit changes all the node tree traversals to use two node > >iteration primitives consistently. These are "first_child" and > >"next_child", both of which automatically skip text nodes. They > >replace the existing "advance_to_next_sibling_element" and *all* > >direct references to "children" and "next" nodes. The unwanted > >dependency on text within elements has been fixed. > > > >For-loops constructed with the new primitives present only element > >nodes to the loop body. This allows a lot of redundant checking for > >null / non-element nodes to be removed. > > > >These changes have been tested by using abidiff to comparing all > >current test XML files with copies where all text has been removed. > >One such test case is included in this commit. > > > > * include/abg-libxml-utils.h > > (advance_to_next_sibling_element): Remove this function. > > (first_child): Add new function. (next_child): Likewise. This > > is simpler than but functionally identical to > > advance_to_next_sibling_element. > > * src/abg-libxml-utils.cc (skip_non_elements): New function to > > skip non-element nodes in a sibling node list. (first_child): > > New function to find the first child element of a node. > > (next_child): New function to find the next sibling element of > > a node. > > * src/abg-reader.cc (walk_xml_node_to_map_type_ids): Remove > > redundant node checks. Iterate over children with first_child > > and next_child helpers. (read_translation_unit): Likewise. > > (read_translation_unit_from_input): Remove a no-longer-needed > > ->next and redundant node checks. Advance "corpus node" after > > reading the translation unit. (read_symbol_db_from_input): > > Remove a no-longer-needed ->next and advance "corpus node" as > > part of for-loop. Remove redundant node checks. > > (build_needed): Remove redundant node checks. Iterate over > > children with first_child and next_child. > > (read_elf_needed_from_input): Remove redundant node checks and > > simplify logic. Move to next element node after reading > > elf-needed node. (read_corpus_from_input): Remove no-children > > early return (which was inhibited by text nodes anyway). Set > > initial "corpus node" using first_child instead of children > > pointer. Remove redundant "corpus node" updates at end of > > function as the caller takes care of this and the node can > > become null with the new primitives. > > (read_corpus_group_from_input): Iterate over children using > > first_child and next child. Set "corpus node" for each corpus, > > not just the first. Incidentally keep going even if one corpus > > parse fails. (build_namespace_decl): Remove redundant node > > checks. Iterate over children using first_child and > > next_child. (build_elf_symbol): Remove redundant node checks. > > (build_elf_symbol_db): Iterate over children using first_child > > and next_child. (build_function_decl): Remove redundant node > > checks. Iterate over children using first_child and > > next_child. (build_function_type): Likewise. > > (build_array_type_def): Likewise. (build_enum_type_decl): > > Likewise. (build_class_decl): Likewise. (build_union_decl): > > Likewise. (build_function_tdecl): Likewise. > > (build_type_composition): Likewise. > > (build_template_tparameter): Likewise. > > (tests/data/Makefile.am): Add new test files. > > (tests/data/test-abidiff-exit/test-squished-report.txt): Add > > empty diff report. > > (tests/data/test-abidiff-exit/test-squished-v0.abi): Add test > > ABI file, containing all ELF elements. > > (tests/data/test-abidiff-exit/test-squished-v1.abi): Likewise > > and identical, except that all text has been stripped. > > (tests/test-abidiff-exit.cc): Add new test case. > > > >Signed-off-by: Giuliano Procida > > That is a very nice simplification that comes with this fix! Thanks! > > Cheers. v2 on the way. Giuliano. > Reviewed-by: Matthias Maennich > > Cheers, > Matthias > > >--- > > include/abg-libxml-utils.h | 5 +- > > src/abg-libxml-utils.cc | 47 ++-- > > src/abg-reader.cc | 208 +++++------------- > > tests/data/Makefile.am | 3 + > > .../test-squished-report.txt | 0 > > .../test-abidiff-exit/test-squished-v0.abi | 43 ++++ > > .../test-abidiff-exit/test-squished-v1.abi | 1 + > > tests/test-abidiff-exit.cc | 11 + > > 8 files changed, 136 insertions(+), 182 deletions(-) > > create mode 100644 tests/data/test-abidiff-exit/test-squished-report.txt > > create mode 100644 tests/data/test-abidiff-exit/test-squished-v0.abi > > create mode 100644 tests/data/test-abidiff-exit/test-squished-v1.abi > > > >diff --git a/include/abg-libxml-utils.h b/include/abg-libxml-utils.h > >index 69e940f6..f87ee5a0 100644 > >--- a/include/abg-libxml-utils.h > >+++ b/include/abg-libxml-utils.h > >@@ -82,7 +82,10 @@ int get_xml_node_depth(xmlNodePtr); > > reinterpret_cast(xml_char_str.get()) > > > > xmlNodePtr > >-advance_to_next_sibling_element(xmlNodePtr node); > >+first_child(xmlNodePtr node); > >+ > >+xmlNodePtr > >+next_child(xmlNodePtr node); > > > > void > > escape_xml_string(const std::string& str, > >diff --git a/src/abg-libxml-utils.cc b/src/abg-libxml-utils.cc > >index a1acff1f..baea25ac 100644 > >--- a/src/abg-libxml-utils.cc > >+++ b/src/abg-libxml-utils.cc > >@@ -413,40 +413,39 @@ unescape_xml_comment(const std::string& str) > > return result; > > } > > > >-/// Maybe get the next sibling element node of an XML node, or stay to > the sam > >+/// Skip until we reach an XML element node or run out of (child) nodes. > > /// > >-/// If there is no next sibling xml element node, the function returns > >-/// the initial node. > >+/// @param a pointer to a node. > > /// > >-/// @param node the initial node to consider. > >-/// > >-/// @return the next sibling node or the initial node @p node. > >-static xmlNodePtr > >-go_to_next_sibling_element_or_stay(xmlNodePtr node) > >+/// @return a pointer to an XML element node or nil. > >+xmlNodePtr > >+skip_non_elements(xmlNodePtr node) > > { > >- xmlNodePtr n; > >- for (n = node; n; n = n->next) > >- { > >- if (n->type == XML_ELEMENT_NODE) > >- break; > >- } > >- return n ? n : node; > >+ while (node && node->type != XML_ELEMENT_NODE) > >+ node = node->next; > >+ return node; > > } > > > >-/// Get the next sibling element node of an XML node. > >+/// Get the first child element of an XML node. > > /// > >-/// If there is no next sibling xml element node, the function returns > nil. > >+/// @param a pointer to the node whose children are to be perused. > > /// > >-/// @param node the XML node to consider. > >+/// @return a pointer to the first XML element child of @p node or nil. > >+xmlNodePtr > >+first_child(xmlNodePtr node) > >+{ > >+ return skip_non_elements(node->children); > >+} > >+ > >+/// Get the next sibling element of an XML node. > > /// > >-/// @return the next sibling element node or nil. > >+/// @param a pointer to the XML node whose following siblings are to be > perused. > >+/// > >+/// @return a pointer to the next sibling element of @p node or nil. > > xmlNodePtr > >-advance_to_next_sibling_element(xmlNodePtr node) > >+next_child(xmlNodePtr node) > > { > >- xmlNodePtr n = go_to_next_sibling_element_or_stay(node->next); > >- if (n == 0 || n->type != XML_ELEMENT_NODE) > >- return 0; > >- return n; > >+ return skip_non_elements(node->next); > > } > > > > }//end namespace xml > >diff --git a/src/abg-reader.cc b/src/abg-reader.cc > >index 3e552864..a143eb58 100644 > >--- a/src/abg-reader.cc > >+++ b/src/abg-reader.cc > >@@ -1343,18 +1343,13 @@ static void > > walk_xml_node_to_map_type_ids(read_context& ctxt, > > xmlNodePtr node) > > { > >- xmlNodePtr n = node; > >- > >- if (!n || n->type != XML_ELEMENT_NODE) > >- return; > >- > >- if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(n, "id")) > >+ if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "id")) > > { > > string id = CHAR_STR(s); > >- ctxt.map_id_and_node(id, n); > >+ ctxt.map_id_and_node(id, node); > > } > > > >- for (n = n->children; n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > walk_xml_node_to_map_type_ids(ctxt, n); > > } > > > >@@ -1396,12 +1391,8 @@ read_translation_unit(read_context& ctxt, > translation_unit& tu, xmlNodePtr node) > > || !ctxt.get_corpus()) > > walk_xml_node_to_map_type_ids(ctxt, node); > > > >- for (xmlNodePtr n = node->children; n; n = n->next) > >- { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- handle_element_node(ctxt, n, /*add_decl_to_scope=*/true); > >- } > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > >+ handle_element_node(ctxt, n, /*add_decl_to_scope=*/true); > > > > ctxt.pop_scope_or_abort(tu.get_global_scope()); > > > >@@ -1495,16 +1486,9 @@ read_translation_unit_from_input(read_context& > ctxt) > > else > > { > > node = 0; > >- for (xmlNodePtr n = ctxt.get_corpus_node()->next; n; n = n->next) > >- { > >- if (!n > >- || n->type != XML_ELEMENT_NODE) > >- continue; > >- if (!xmlStrEqual(n->name, BAD_CAST("abi-instr"))) > >- return nil; > >- node = n; > >- break; > >- } > >+ xmlNodePtr n = ctxt.get_corpus_node(); > >+ if (n && xmlStrEqual(n->name, BAD_CAST("abi-instr"))) > >+ node = n; > > } > > > > if (node == 0) > >@@ -1517,9 +1501,9 @@ read_translation_unit_from_input(read_context& > ctxt) > > // case, after that unexpected call to read_translation_unit(), the > > // current corpus node of the context is going to point to that > > // translation unit that has been read under the hood. Let's set > >- // the corpus node to the one we initially called > >+ // the corpus node to just after the one we initially called > > // read_translation_unit() on here. > >- ctxt.set_corpus_node(node); > >+ ctxt.set_corpus_node(xml::next_child(node)); > > return tu; > > } > > > >@@ -1593,11 +1577,8 @@ read_symbol_db_from_input(read_context& > ctxt, > > xmlTextReaderNext(reader.get()); > > } > > else > >- for (xmlNodePtr n = ctxt.get_corpus_node()->next; n; n = n->next) > >+ for (xmlNodePtr n = ctxt.get_corpus_node(); n; n = > xml::next_child(n), ctxt.set_corpus_node(n)) > > That line looks a little long. > > It makes sense to only update the "corpus node" once, after the loop completes. I've restructured it and indentation has been reduced as a side effect. > > { > >- if (!n || n->type != XML_ELEMENT_NODE) > >- continue; > >- > > bool has_fn_syms = false, has_var_syms = false; > > if (xmlStrEqual(n->name, BAD_CAST("elf-function-symbols"))) > > has_fn_syms = true; > >@@ -1605,7 +1586,6 @@ read_symbol_db_from_input(read_context& > ctxt, > > has_var_syms = true; > > else > > break; > >- ctxt.set_corpus_node(n); > > if (has_fn_syms) > > { > > fn_symdb = build_elf_symbol_db(ctxt, n, true); > >@@ -1636,16 +1616,12 @@ read_symbol_db_from_input(read_context& > ctxt, > > static bool > > build_needed(xmlNode* node, vector& needed) > > { > >- if (!node) > >- return false; > >- > >- if (!node || !xmlStrEqual(node->name,BAD_CAST("elf-needed"))) > >+ if (!xmlStrEqual(node->name,BAD_CAST("elf-needed"))) > > return false; > > > >- for (xmlNodePtr n = node->children; n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > >- if (n->type != XML_ELEMENT_NODE > >- || !xmlStrEqual(n->name, BAD_CAST("dependency"))) > >+ if (!xmlStrEqual(n->name, BAD_CAST("dependency"))) > > continue; > > > > string name; > >@@ -1693,27 +1669,19 @@ read_elf_needed_from_input(read_context& > ctxt, > > return false; > > > > node = xmlTextReaderExpand(reader.get()); > >- if (!node) > >- return false; > > } > > else > > { > >- for (xmlNodePtr n = ctxt.get_corpus_node()->next; n; n = n->next) > >- { > >- if (!n || n->type != XML_ELEMENT_NODE) > >- continue; > >- if (!xmlStrEqual(n->name, BAD_CAST("elf-needed"))) > >- return false; > >- node = n; > >- break; > >- } > >+ xmlNodePtr n = ctxt.get_corpus_node(); > >+ if (n && xmlStrEqual(n->name, BAD_CAST("elf-needed"))) > >+ node = n; > > } > > > > bool result = false; > > if (node) > > { > > result = build_needed(node, needed); > >- ctxt.set_corpus_node(node); > >+ ctxt.set_corpus_node(xml::next_child(node)); > > } > > > > return result; > >@@ -1917,10 +1885,7 @@ read_corpus_from_input(read_context& ctxt) > > corp.set_soname(reinterpret_cast(soname_str.get())); > > } > > > >- if (!node->children) > >- return nil; > >- > >- ctxt.set_corpus_node(node->children); > >+ ctxt.set_corpus_node(xml::first_child(node)); > > > > corpus& corp = *ctxt.get_corpus(); > > > >@@ -1986,17 +1951,6 @@ read_corpus_from_input(read_context& ctxt) > > // call at the beginning of the function. > > xmlTextReaderNext(reader.get()); > > } > >- else > >- { > >- node = ctxt.get_corpus_node(); > >- node = xml::advance_to_next_sibling_element(node); > >- if (!node) > >- { > >- node = ctxt.get_corpus_node(); > >- node = xml::advance_to_next_sibling_element(node->parent); > >- } > >- ctxt.set_corpus_node(node); > >- } > > > > return ctxt.get_corpus(); > > } > >@@ -2046,13 +2000,13 @@ read_corpus_group_from_input(read_context& ctxt) > > if (!node) > > return nil; > > > >- //node = xml::get_first_element_sibling_if_text(node->children); > >- node = xml::advance_to_next_sibling_element(node->children); > >- ctxt.set_corpus_node(node); > >- > >- corpus_sptr corp; > >- while ((corp = read_corpus_from_input(ctxt))) > >- ctxt.get_corpus_group()->add_corpus(corp); > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > >+ { > >+ ctxt.set_corpus_node(n); > >+ corpus_sptr corp = read_corpus_from_input(ctxt); > >+ if (corp) > >+ ctxt.get_corpus_group()->add_corpus(corp); > >+ } > > > > xmlTextReaderNext(reader.get()); > > > >@@ -2738,12 +2692,8 @@ build_namespace_decl(read_context& ctxt, > > ctxt.push_decl_to_current_scope(decl, add_to_current_scope); > > ctxt.map_xml_node_to_decl(node, decl); > > > >- for (xmlNodePtr n = node->children; n; n = n->next) > >- { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- handle_element_node(ctxt, n, /*add_to_current_scope=*/true); > >- } > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > >+ handle_element_node(ctxt, n, /*add_to_current_scope=*/true); > > > > ctxt.pop_scope_or_abort(decl); > > > >@@ -2767,9 +2717,7 @@ build_elf_symbol(read_context& ctxt, const > xmlNodePtr node, > > { > > elf_symbol_sptr nil; > > > >- if (!node > >- || node->type != XML_ELEMENT_NODE > >- || !xmlStrEqual(node->name, BAD_CAST("elf-symbol"))) > >+ if (!xmlStrEqual(node->name, BAD_CAST("elf-symbol"))) > > return nil; > > > > string name; > >@@ -2928,7 +2876,7 @@ build_elf_symbol_db(read_context& ctxt, > > xml_node_ptr_elf_symbol_sptr_map_type xml_node_ptr_elf_symbol_map; > > > > elf_symbol_sptr sym; > >- for (xmlNodePtr n = node->children; n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > > if ((sym = build_elf_symbol(ctxt, n, /*drop_if_suppress=*/true))) > > { > >@@ -3094,12 +3042,9 @@ build_function_decl(read_context& ctxt, > > std::vector parms; > > type_base_sptr return_type = env->get_void_type(); > > > >- for (xmlNodePtr n = node->children; n ; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > >- else if (xmlStrEqual(n->name, BAD_CAST("parameter"))) > >+ if (xmlStrEqual(n->name, BAD_CAST("parameter"))) > > { > > if (function_decl::parameter_sptr p = > > build_function_parameter(ctxt, n)) > >@@ -3789,12 +3734,9 @@ build_function_type(read_context& ctxt, > > ctxt.get_translation_unit()->bind_function_type_life_time(fn_type); > > ctxt.key_type_decl(fn_type, id); > > > >- for (xmlNodePtr n = node->children; n ; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > >- else if (xmlStrEqual(n->name, BAD_CAST("parameter"))) > >+ if (xmlStrEqual(n->name, BAD_CAST("parameter"))) > > { > > if (function_decl::parameter_sptr p = > > build_function_parameter(ctxt, n)) > >@@ -4025,12 +3967,9 @@ build_array_type_def(read_context& ctxt, > > read_location(ctxt, node, loc); > > array_type_def::subranges_type subranges; > > > >- for (xmlNodePtr n = node->children; n ; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > >- else if (xmlStrEqual(n->name, BAD_CAST("subrange"))) > >+ if (xmlStrEqual(n->name, BAD_CAST("subrange"))) > > { > > if (array_type_def::subrange_sptr s = > > build_subrange_type(ctxt, n)) > >@@ -4183,11 +4122,8 @@ build_enum_type_decl(read_context& ctxt, > > > > string base_type_id; > > enum_type_decl::enumerators enums; > >- for (xmlNodePtr n = node->children; n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (xmlStrEqual(n->name, BAD_CAST("underlying-type"))) > > { > > xml_char_sptr a = xml::build_sptr(xmlGetProp(n, > BAD_CAST("type-id"))); > >@@ -4554,11 +4490,8 @@ build_class_decl(read_context& ctxt, > > decl->set_naming_typedef(naming_typedef); > > } > > > >- for (xmlNodePtr n = node->children; !is_decl_only && n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); !is_decl_only && n; n = > xml::next_child(n)) > > Likewise. > > I've written a very brief note at https://sourceware.org/bugzilla/show_bug.cgi?id=26591#c6 and made the skipping of child elements much more obvious. (And the same for the other case.) > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (xmlStrEqual(n->name, BAD_CAST("base-class"))) > > { > > access_specifier access = > >@@ -4605,11 +4538,8 @@ build_class_decl(read_context& ctxt, > > > > ctxt.map_xml_node_to_decl(n, decl); > > > >- for (xmlNodePtr p = n->children; p; p = p->next) > >+ for (xmlNodePtr p = xml::first_child(n); p; p = > xml::next_child(p)) > > { > >- if (p->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (type_base_sptr t = > > build_type(ctxt, p, /*add_to_current_scope=*/true)) > > { > >@@ -4643,11 +4573,8 @@ build_class_decl(read_context& ctxt, > > bool is_static = false; > > read_static(n, is_static); > > > >- for (xmlNodePtr p = n->children; p; p = p->next) > >+ for (xmlNodePtr p = xml::first_child(n); p; p = > xml::next_child(p)) > > { > >- if (p->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (var_decl_sptr v = > > build_var_decl(ctxt, p, /*add_to_cur_scope=*/false)) > > { > >@@ -4701,11 +4628,8 @@ build_class_decl(read_context& ctxt, > > bool is_ctor = false, is_dtor = false, is_const = false; > > read_cdtor_const(n, is_ctor, is_dtor, is_const); > > > >- for (xmlNodePtr p = n->children; p; p = p->next) > >+ for (xmlNodePtr p = xml::first_child(n); p; p = > xml::next_child(p)) > > { > >- if (p->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (function_decl_sptr f = > > build_function_decl_if_not_suppressed(ctxt, p, decl, > > > /*add_to_cur_sc=*/true)) > >@@ -4740,11 +4664,8 @@ build_class_decl(read_context& ctxt, > > bool is_ctor = false, is_dtor = false, is_const = false; > > read_cdtor_const(n, is_ctor, is_dtor, is_const); > > > >- for (xmlNodePtr p = n->children; p; p = p->next) > >+ for (xmlNodePtr p = xml::first_child(n); p; p = > xml::next_child(p)) > > { > >- if (p->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (shared_ptr f = > > build_function_tdecl(ctxt, p, > > /*add_to_current_scope=*/true)) > >@@ -4954,11 +4875,8 @@ build_union_decl(read_context& ctxt, > > ctxt.map_xml_node_to_decl(node, decl); > > ctxt.key_type_decl(decl, id); > > > >- for (xmlNodePtr n = node->children; !is_decl_only && n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); !is_decl_only && n; n = > xml::next_child(n)) > > Likewise. > > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (xmlStrEqual(n->name, BAD_CAST("member-type"))) > > { > > access_specifier access = private_access; > >@@ -4966,11 +4884,8 @@ build_union_decl(read_context& ctxt, > > > > ctxt.map_xml_node_to_decl(n, decl); > > > >- for (xmlNodePtr p = n->children; p; p = p->next) > >+ for (xmlNodePtr p = xml::first_child(n); p; p = > xml::next_child(p)) > > { > >- if (p->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (type_base_sptr t = > > build_type(ctxt, p, /*add_to_current_scope=*/true)) > > { > >@@ -4998,11 +4913,8 @@ build_union_decl(read_context& ctxt, > > bool is_static = false; > > read_static(n, is_static); > > > >- for (xmlNodePtr p = n->children; p; p = p->next) > >+ for (xmlNodePtr p = xml::first_child(n); p; p = > xml::next_child(p)) > > { > >- if (p->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (var_decl_sptr v = > > build_var_decl(ctxt, p, /*add_to_cur_scope=*/false)) > > { > >@@ -5040,11 +4952,8 @@ build_union_decl(read_context& ctxt, > > bool is_ctor = false, is_dtor = false, is_const = false; > > read_cdtor_const(n, is_ctor, is_dtor, is_const); > > > >- for (xmlNodePtr p = n->children; p; p = p->next) > >+ for (xmlNodePtr p = xml::first_child(n); p; p = > xml::next_child(p)) > > { > >- if (p->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (function_decl_sptr f = > > build_function_decl_if_not_suppressed(ctxt, p, decl, > > > /*add_to_cur_sc=*/true)) > >@@ -5073,11 +4982,8 @@ build_union_decl(read_context& ctxt, > > bool is_ctor = false, is_dtor = false, is_const = false; > > read_cdtor_const(n, is_ctor, is_dtor, is_const); > > > >- for (xmlNodePtr p = n->children; p; p = p->next) > >+ for (xmlNodePtr p = xml::first_child(n); p; p = > xml::next_child(p)) > > { > >- if (p->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (function_tdecl_sptr f = > > build_function_tdecl(ctxt, p, > > /*add_to_current_scope=*/true)) > >@@ -5152,11 +5058,8 @@ build_function_tdecl(read_context& ctxt, > > ctxt.push_decl_to_current_scope(fn_tmpl_decl, add_to_current_scope); > > > > unsigned parm_index = 0; > >- for (xmlNodePtr n = node->children; n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (template_parameter_sptr parm = > > build_template_parameter(ctxt, n, parm_index, fn_tmpl_decl)) > > { > >@@ -5216,11 +5119,8 @@ build_class_tdecl(read_context& ctxt, > > ctxt.push_decl_to_current_scope(class_tmpl, add_to_current_scope); > > > > unsigned parm_index = 0; > >- for (xmlNodePtr n = node->children; n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (template_parameter_sptr parm= > > build_template_parameter(ctxt, n, parm_index, class_tmpl)) > > { > >@@ -5335,11 +5235,8 @@ build_type_composition(read_context& > ctxt, > > > ctxt.push_decl_to_current_scope(dynamic_pointer_cast(result), > > /*add_to_current_scope=*/true); > > > >- for (xmlNodePtr n = node->children; n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > > if ((composed_type = > > build_pointer_type_def(ctxt, n, > > /*add_to_current_scope=*/true)) > >@@ -5465,11 +5362,8 @@ build_template_tparameter(read_context& ctxt, > > > > // Go parse template parameters that are children nodes > > int parm_index = 0; > >- for (xmlNodePtr n = node->children; n; n = n->next) > >+ for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n)) > > { > >- if (n->type != XML_ELEMENT_NODE) > >- continue; > >- > > if (shared_ptr p = > > build_template_parameter(ctxt, n, parm_index, result)) > > { > >diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am > >index 4fb2d6a4..7bdae7bf 100644 > >--- a/tests/data/Makefile.am > >+++ b/tests/data/Makefile.am > >@@ -197,6 +197,9 @@ test-abidiff-exit/test-non-leaf-array-v0.o \ > > test-abidiff-exit/test-non-leaf-array-v1.c \ > > test-abidiff-exit/test-non-leaf-array-v1.o \ > > test-abidiff-exit/test-non-leaf-array-report.txt \ > >+test-abidiff-exit/test-squished-v0.abi \ > >+test-abidiff-exit/test-squished-v1.abi \ > >+test-abidiff-exit/test-squished-report.txt \ > > \ > > test-diff-dwarf/test0-v0.cc \ > > test-diff-dwarf/test0-v0.o \ > >diff --git a/tests/data/test-abidiff-exit/test-squished-report.txt > b/tests/data/test-abidiff-exit/test-squished-report.txt > >new file mode 100644 > >index 00000000..e69de29b > >diff --git a/tests/data/test-abidiff-exit/test-squished-v0.abi > b/tests/data/test-abidiff-exit/test-squished-v0.abi > >new file mode 100644 > >index 00000000..6b3d0460 > >--- /dev/null > >+++ b/tests/data/test-abidiff-exit/test-squished-v0.abi > >@@ -0,0 +1,43 @@ > >+ > >+ > >+ > >+ > >+ > >+ > >+ > >+ > >+ visibility='default-visibility' is-defined='yes'/> > >+ binding='global-binding' visibility='default-visibility' is-defined='yes'/> > >+ binding='weak-binding' visibility='default-visibility' is-defined='yes'/> > >+ visibility='default-visibility' is-defined='yes'/> > >+ visibility='default-visibility' is-defined='yes'/> > >+ > >+ > >+ binding='gnu-unique-binding' visibility='default-visibility' > is-defined='yes'/> > >+ binding='gnu-unique-binding' visibility='default-visibility' > is-defined='yes'/> > >+ > >+ comp-dir-path='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf' > language='LANG_C_plus_plus'> > >+ > >+ visibility='default' > filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='9' column='1' id='type-id-2'> > >+ > >+ filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='11' column='1' visibility='default' binding='global' > size-in-bits='64' elf-symbol-id='_ZN1B3fooEv'> > >+ is-artificial='yes'/> > >+ > >+ > >+ > >+ > >+ visibility='default' > filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='26' column='1' id='type-id-4'> > >+ > >+ mangled-name='_ZN1CIiE3barE' visibility='default' > filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='31' column='1' elf-symbol-id='_ZN1CIiE3barE'/> > >+ > >+ > >+ id='type-id-5'/> > >+ > >+ filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='19' column='1' visibility='default' binding='global' > size-in-bits='64' elf-symbol-id='_Z3barv'> > >+ > >+ > >+ filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='34' column='1' visibility='default' binding='global' > size-in-bits='64' elf-symbol-id='_Z4blehv'> > >+ > >+ > >+ > >+ > >diff --git a/tests/data/test-abidiff-exit/test-squished-v1.abi > b/tests/data/test-abidiff-exit/test-squished-v1.abi > >new file mode 100644 > >index 00000000..3ffa272b > >--- /dev/null > >+++ b/tests/data/test-abidiff-exit/test-squished-v1.abi > >@@ -0,0 +1 @@ > >+ path='data/test-read-dwarf/test6.so'> name='libstdc++.so.6'/> name='libgcc_s.so.1'/> name='libc.so.6'/> name='_Z3barv' type='func-type' binding='global-binding' > visibility='default-visibility' is-defined='yes'/> name='_Z4blehv' type='func-type' binding='global-binding' > visibility='default-visibility' is-defined='yes'/> name='_ZN1B3fooEv' type='func-type' binding='weak-binding' > visibility='default-visibility' is-defined='yes'/> type='func-type' binding='global-binding' visibility='default-visibility' > is-defined='yes'/> binding='global-binding' visibility='default-visibility' > is-defined='yes'/> name='_ZN1CIiE3barE' size='4' type='object-type' > binding='gnu-unique-binding' visibility='default-visibility' > is-defined='yes'/> type='object-type' binding='gnu-unique-binding' > visibility='default-visibility' > is-defined='yes'/> path='test6.cc' > comp-dir-path='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf' > language='LANG_C_plus_plus'> id='type-id-1'/> visibility='default' > filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='9' column='1' id='type-id-2'> access='public'> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='11' column='1' visibility='default' binding='global' > size-in-bits='64' elf-symbol-id='_ZN1B3fooEv'> type-id='type-id-3' name='this' is-artificial='yes'/> type-id='type-id-1'/> name='C<int>' size-in-bits='8' is-struct='yes' visibility='default' > filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='26' column='1' id='type-id-4'> static='yes'> mangled-name='_ZN1CIiE3barE' visibility='default' > filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='31' column='1' > elf-symbol-id='_ZN1CIiE3barE'/> type-id='type-id-2' size-in-bits='64' id='type-id-5'/> type-id='type-id-5' const='yes' id='type-id-3'/> mangled-name='_Z3barv' > filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='19' column='1' visibility='default' binding='global' > size-in-bits='64' elf-symbol-id='_Z3barv'> type-id='type-id-1'/> mangled-name='_Z4blehv' > filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc' > line='34' column='1' visibility='default' binding='global' > size-in-bits='64' elf-symbol-id='_Z4blehv'> type-id='type-id-1'/> > >\ No newline at end of file > >diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc > >index 4283e145..f1526b9e 100644 > >--- a/tests/test-abidiff-exit.cc > >+++ b/tests/test-abidiff-exit.cc > >@@ -392,6 +392,17 @@ InOutSpec in_out_specs[] = > > "data/test-abidiff-exit/test-non-leaf-array-report.txt", > > "output/test-abidiff-exit/test-non-leaf-array-report.txt" > > }, > >+ { > >+ "data/test-abidiff-exit/test-squished-v0.abi", > >+ "data/test-abidiff-exit/test-squished-v1.abi", > >+ "", > >+ "", > >+ "", > >+ "--harmless", > >+ abigail::tools_utils::ABIDIFF_OK, > >+ "data/test-abidiff-exit/test-squished-report.txt", > >+ "output/test-abidiff-exit/test-squished-report.txt" > >+ }, > > {0, 0, 0 ,0, 0, 0, abigail::tools_utils::ABIDIFF_OK, 0, 0} > > }; > > > >-- > >2.31.0.291.g576ba9dcdaf-goog > > >