From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by sourceware.org (Postfix) with ESMTPS id CC7983847809 for ; Tue, 20 Apr 2021 09:46:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CC7983847809 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dodji@seketeli.org Received: from localhost (unknown [88.120.130.27]) (Authenticated sender: dodji@seketeli.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id E1175240005; Tue, 20 Apr 2021 09:46:21 +0000 (UTC) Received: by localhost (Postfix, from userid 1000) id 5E8C158000E; Tue, 20 Apr 2021 11:46:21 +0200 (CEST) From: Dodji Seketeli To: Giuliano Procida Cc: Matthias =?utf-8?Q?M=C3=A4nnich?= , Giuliano Procida via Libabigail , kernel-team@android.com Subject: Re: [PATCH 3/3] reader: Use xmlFirstElementChild/xmlNextElementSibling to iterate over children elements Organization: Me, myself and I References: <20210331092352.619148-1-gprocida@google.com> <20210331170454.951900-1-gprocida@google.com> <8735w3vdkv.fsf@seketeli.org> <87o8eftz5v.fsf_-_@seketeli.org> <87k0oytmsf.fsf@seketeli.org> <877dkytlqo.fsf_-_@seketeli.org> X-Operating-System: Fedora 34 X-URL: http://www.seketeli.net/~dodji Date: Tue, 20 Apr 2021 11:46:21 +0200 In-Reply-To: (Giuliano Procida's message of "Tue, 20 Apr 2021 07:52:52 +0100") Message-ID: <8735vlthjm.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: Tue, 20 Apr 2021 09:46:35 -0000 Hello Giuliano, Giuliano Procida a =C3=A9crit: > I had a quick look and there's (at least) one place where there still a > direct access to ->children. The code works because there are separate > tests to not process wrong nodes, but it would be neater to use the first > element child function uniformly (as well). You are right. Below is an update of that patch that hopefully addresses your comment. Thanks. >From 0a6c322f6ec8be354122c6b4cdefe6143eb568ed Mon Sep 17 00:00:00 2001 From: Dodji Seketeli Date: Mon, 19 Apr 2021 13:50:33 +0200 Subject: [PATCH 3/3] reader: Use xmlFirstElementChild/xmlNextElementSibling= to iterate over children elements Use xmlFirstElementChild/xmlNextElementSibling to iterate over element children nodes rather than doing it by hand in the various for loops. * src/abg-reader.cc (walk_xml_node_to_map_type_ids) (read_translation_unit, read_translation_unit_from_input) (read_symbol_db_from_input, build_needed) (read_elf_needed_from_input, read_corpus_group_from_input) (build_namespace_decl, build_elf_symbol_db, build_function_decl) (build_function_type, build_array_type_def, build_enum_type_decl) (build_class_decl, build_union_decl, build_function_tdecl) (build_class_tdecl, build_type_composition) (build_template_tparameter): Use xmlFirstElementChild/xmlNextElementSibling rather than poking at xmlNode::children and looping over xmlNode::next by hand. Signed-off-by: Dodji Seketeli --- src/abg-reader.cc | 239 ++++++++++++++++++++-------------------------- 1 file changed, 103 insertions(+), 136 deletions(-) diff --git a/src/abg-reader.cc b/src/abg-reader.cc index f5ed87b2..39629314 100644 --- a/src/abg-reader.cc +++ b/src/abg-reader.cc @@ -1354,7 +1354,7 @@ walk_xml_node_to_map_type_ids(read_context& ctxt, ctxt.map_id_and_node(id, n); } =20 - for (n =3D n->children; n; n =3D n->next) + for (n =3D xmlFirstElementChild(n); n; n =3D xmlNextElementSibling(n)) walk_xml_node_to_map_type_ids(ctxt, n); } =20 @@ -1396,12 +1396,10 @@ read_translation_unit(read_context& ctxt, translati= on_unit& tu, xmlNodePtr node) || !ctxt.get_corpus()) walk_xml_node_to_map_type_ids(ctxt, node); =20 - for (xmlNodePtr n =3D node->children; n; n =3D n->next) - { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - handle_element_node(ctxt, n, /*add_decl_to_scope=3D*/true); - } + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) + handle_element_node(ctxt, n, /*add_decl_to_scope=3D*/true); =20 ctxt.pop_scope_or_abort(tu.get_global_scope()); =20 @@ -1495,11 +1493,10 @@ read_translation_unit_from_input(read_context& ctxt) else { node =3D 0; - for (xmlNodePtr n =3D ctxt.get_corpus_node(); n; n =3D n->next) + for (xmlNodePtr n =3D ctxt.get_corpus_node(); + n; + n =3D xmlNextElementSibling(n)) { - if (!n - || n->type !=3D XML_ELEMENT_NODE) - continue; if (!xmlStrEqual(n->name, BAD_CAST("abi-instr"))) return nil; node =3D n; @@ -1595,11 +1592,8 @@ read_symbol_db_from_input(read_context& ctxt, xmlTextReaderNext(reader.get()); } else - for (xmlNodePtr n =3D ctxt.get_corpus_node(); n; n =3D n->next) + for (xmlNodePtr n =3D ctxt.get_corpus_node(); n; n =3D xmlNextElementS= ibling(n)) { - if (!n || n->type !=3D XML_ELEMENT_NODE) - continue; - bool has_fn_syms =3D false, has_var_syms =3D false; if (xmlStrEqual(n->name, BAD_CAST("elf-function-symbols"))) has_fn_syms =3D true; @@ -1641,16 +1635,14 @@ 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"))) return false; =20 - for (xmlNodePtr n =3D node->children; n; n =3D n->next) + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) { - if (n->type !=3D XML_ELEMENT_NODE - || !xmlStrEqual(n->name, BAD_CAST("dependency"))) + if (!xmlStrEqual(n->name, BAD_CAST("dependency"))) continue; =20 string name; @@ -1703,10 +1695,10 @@ read_elf_needed_from_input(read_context& ctxt, } else { - for (xmlNodePtr n =3D ctxt.get_corpus_node(); n; n =3D n->next) + for (xmlNodePtr n =3D ctxt.get_corpus_node(); + n; + n =3D xmlNextElementSibling(n)) { - if (!n || n->type !=3D XML_ELEMENT_NODE) - continue; if (!xmlStrEqual(n->name, BAD_CAST("elf-needed"))) return false; node =3D n; @@ -2054,7 +2046,6 @@ read_corpus_group_from_input(read_context& ctxt) if (!node) return nil; =20 - //node =3D xml::get_first_element_sibling_if_text(node->children); node =3D xmlFirstElementChild(node); ctxt.set_corpus_node(node); =20 @@ -2746,12 +2737,10 @@ 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); =20 - for (xmlNodePtr n =3D node->children; n; n =3D n->next) - { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - handle_element_node(ctxt, n, /*add_to_current_scope=3D*/true); - } + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) + handle_element_node(ctxt, n, /*add_to_current_scope=3D*/true); =20 ctxt.pop_scope_or_abort(decl); =20 @@ -2938,14 +2927,14 @@ build_elf_symbol_db(read_context& ctxt, xml_node_ptr_elf_symbol_sptr_map_type xml_node_ptr_elf_symbol_map; =20 elf_symbol_sptr sym; - for (xmlNodePtr n =3D node->children; n; n =3D n->next) - { - if ((sym =3D build_elf_symbol(ctxt, n, /*drop_if_suppress=3D*/false)= )) - { - id_sym_map[sym->get_id_string()] =3D sym; - xml_node_ptr_elf_symbol_map[n] =3D sym; - } - } + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) + if ((sym =3D build_elf_symbol(ctxt, n, /*drop_if_suppress=3D*/false))) + { + id_sym_map[sym->get_id_string()] =3D sym; + xml_node_ptr_elf_symbol_map[n] =3D sym; + } =20 if (id_sym_map.empty()) return nil; @@ -3104,12 +3093,11 @@ build_function_decl(read_context& ctxt, std::vector parms; type_base_sptr return_type =3D env->get_void_type(); =20 - for (xmlNodePtr n =3D node->children; n ; n =3D n->next) + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n ; + n =3D xmlNextElementSibling(n)) { - if (n->type !=3D 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 =3D build_function_parameter(ctxt, n)) @@ -3797,12 +3785,11 @@ build_function_type(read_context& ctxt, ctxt.get_translation_unit()->bind_function_type_life_time(fn_type); ctxt.key_type_decl(fn_type, id); =20 - for (xmlNodePtr n =3D node->children; n ; n =3D n->next) + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) { - if (n->type !=3D 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 =3D build_function_parameter(ctxt, n)) @@ -4033,25 +4020,22 @@ build_array_type_def(read_context& ctxt, read_location(ctxt, node, loc); array_type_def::subranges_type subranges; =20 - for (xmlNodePtr n =3D node->children; n ; n =3D n->next) - { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - - else if (xmlStrEqual(n->name, BAD_CAST("subrange"))) - { - if (array_type_def::subrange_sptr s =3D - build_subrange_type(ctxt, n)) - { - if (add_to_current_scope) - { - add_decl_to_scope(s, ctxt.get_cur_scope()); - ctxt.maybe_canonicalize_type(s); - } - subranges.push_back(s); - } - } - } + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) + if (xmlStrEqual(n->name, BAD_CAST("subrange"))) + { + if (array_type_def::subrange_sptr s =3D + build_subrange_type(ctxt, n)) + { + if (add_to_current_scope) + { + add_decl_to_scope(s, ctxt.get_cur_scope()); + ctxt.maybe_canonicalize_type(s); + } + subranges.push_back(s); + } + } =20 array_type_def_sptr ar_type(new array_type_def(type, subranges, @@ -4191,11 +4175,10 @@ build_enum_type_decl(read_context& ctxt, =20 string base_type_id; enum_type_decl::enumerators enums; - for (xmlNodePtr n =3D node->children; n; n =3D n->next) + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - if (xmlStrEqual(n->name, BAD_CAST("underlying-type"))) { xml_char_sptr a =3D xml::build_sptr(xmlGetProp(n, BAD_CAST("type-id"))); @@ -4562,11 +4545,10 @@ build_class_decl(read_context& ctxt, decl->set_naming_typedef(naming_typedef); } =20 - for (xmlNodePtr n =3D node->children; !is_decl_only && n; n =3D n->next) + for (xmlNodePtr n =3D xmlFirstElementChild(node); + !is_decl_only && n; + n =3D xmlNextElementSibling(n)) { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - if (xmlStrEqual(n->name, BAD_CAST("base-class"))) { access_specifier access =3D @@ -4613,11 +4595,10 @@ build_class_decl(read_context& ctxt, =20 ctxt.map_xml_node_to_decl(n, decl); =20 - for (xmlNodePtr p =3D n->children; p; p =3D p->next) + for (xmlNodePtr p =3D xmlFirstElementChild(n); + p; + p =3D xmlNextElementSibling(p)) { - if (p->type !=3D XML_ELEMENT_NODE) - continue; - if (type_base_sptr t =3D build_type(ctxt, p, /*add_to_current_scope=3D*/true)) { @@ -4651,11 +4632,10 @@ build_class_decl(read_context& ctxt, bool is_static =3D false; read_static(n, is_static); =20 - for (xmlNodePtr p =3D n->children; p; p =3D p->next) + for (xmlNodePtr p =3D xmlFirstElementChild(n); + p; + p =3D xmlNextElementSibling(p)) { - if (p->type !=3D XML_ELEMENT_NODE) - continue; - if (var_decl_sptr v =3D build_var_decl(ctxt, p, /*add_to_cur_scope=3D*/false)) { @@ -4709,11 +4689,10 @@ build_class_decl(read_context& ctxt, bool is_ctor =3D false, is_dtor =3D false, is_const =3D false; read_cdtor_const(n, is_ctor, is_dtor, is_const); =20 - for (xmlNodePtr p =3D n->children; p; p =3D p->next) + for (xmlNodePtr p =3D xmlFirstElementChild(n); + p; + p =3D xmlNextElementSibling(p)) { - if (p->type !=3D XML_ELEMENT_NODE) - continue; - if (function_decl_sptr f =3D build_function_decl_if_not_suppressed(ctxt, p, decl, /*add_to_cur_sc=3D*/true)) @@ -4748,11 +4727,10 @@ build_class_decl(read_context& ctxt, bool is_ctor =3D false, is_dtor =3D false, is_const =3D false; read_cdtor_const(n, is_ctor, is_dtor, is_const); =20 - for (xmlNodePtr p =3D n->children; p; p =3D p->next) + for (xmlNodePtr p =3D xmlFirstElementChild(n); + p; + p =3D xmlNextElementSibling(p)) { - if (p->type !=3D XML_ELEMENT_NODE) - continue; - if (shared_ptr f =3D build_function_tdecl(ctxt, p, /*add_to_current_scope=3D*/true)) @@ -4962,11 +4940,10 @@ build_union_decl(read_context& ctxt, ctxt.map_xml_node_to_decl(node, decl); ctxt.key_type_decl(decl, id); =20 - for (xmlNodePtr n =3D node->children; !is_decl_only && n; n =3D n->next) + for (xmlNodePtr n =3D xmlFirstElementChild(node); + !is_decl_only && n; + n =3D xmlNextElementSibling(n)) { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - if (xmlStrEqual(n->name, BAD_CAST("member-type"))) { access_specifier access =3D private_access; @@ -4974,11 +4951,10 @@ build_union_decl(read_context& ctxt, =20 ctxt.map_xml_node_to_decl(n, decl); =20 - for (xmlNodePtr p =3D n->children; p; p =3D p->next) + for (xmlNodePtr p =3D xmlFirstElementChild(n); + p; + p =3D xmlNextElementSibling(p)) { - if (p->type !=3D XML_ELEMENT_NODE) - continue; - if (type_base_sptr t =3D build_type(ctxt, p, /*add_to_current_scope=3D*/true)) { @@ -5006,11 +4982,10 @@ build_union_decl(read_context& ctxt, bool is_static =3D false; read_static(n, is_static); =20 - for (xmlNodePtr p =3D n->children; p; p =3D p->next) + for (xmlNodePtr p =3D xmlFirstElementChild(n); + p; + p =3D xmlNextElementSibling(p)) { - if (p->type !=3D XML_ELEMENT_NODE) - continue; - if (var_decl_sptr v =3D build_var_decl(ctxt, p, /*add_to_cur_scope=3D*/false)) { @@ -5048,11 +5023,10 @@ build_union_decl(read_context& ctxt, bool is_ctor =3D false, is_dtor =3D false, is_const =3D false; read_cdtor_const(n, is_ctor, is_dtor, is_const); =20 - for (xmlNodePtr p =3D n->children; p; p =3D p->next) + for (xmlNodePtr p =3D xmlFirstElementChild(n); + p; + p =3D xmlNextElementSibling(p)) { - if (p->type !=3D XML_ELEMENT_NODE) - continue; - if (function_decl_sptr f =3D build_function_decl_if_not_suppressed(ctxt, p, decl, /*add_to_cur_sc=3D*/true)) @@ -5081,11 +5055,10 @@ build_union_decl(read_context& ctxt, bool is_ctor =3D false, is_dtor =3D false, is_const =3D false; read_cdtor_const(n, is_ctor, is_dtor, is_const); =20 - for (xmlNodePtr p =3D n->children; p; p =3D p->next) + for (xmlNodePtr p =3D xmlFirstElementChild(n); + p; + p =3D xmlNextElementSibling(p)) { - if (p->type !=3D XML_ELEMENT_NODE) - continue; - if (function_tdecl_sptr f =3D build_function_tdecl(ctxt, p, /*add_to_current_scope=3D*/true)) @@ -5160,11 +5133,10 @@ build_function_tdecl(read_context& ctxt, ctxt.push_decl_to_current_scope(fn_tmpl_decl, add_to_current_scope); =20 unsigned parm_index =3D 0; - for (xmlNodePtr n =3D node->children; n; n =3D n->next) + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - if (template_parameter_sptr parm =3D build_template_parameter(ctxt, n, parm_index, fn_tmpl_decl)) { @@ -5224,11 +5196,10 @@ build_class_tdecl(read_context& ctxt, ctxt.push_decl_to_current_scope(class_tmpl, add_to_current_scope); =20 unsigned parm_index =3D 0; - for (xmlNodePtr n =3D node->children; n; n =3D n->next) + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - if (template_parameter_sptr parm=3D build_template_parameter(ctxt, n, parm_index, class_tmpl)) { @@ -5343,11 +5314,10 @@ build_type_composition(read_context& ctxt, ctxt.push_decl_to_current_scope(dynamic_pointer_cast(result), /*add_to_current_scope=3D*/true); =20 - for (xmlNodePtr n =3D node->children; n; n =3D n->next) + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - if ((composed_type =3D build_pointer_type_def(ctxt, n, /*add_to_current_scope=3D*/true)) @@ -5473,18 +5443,15 @@ build_template_tparameter(read_context& ctxt, =20 // Go parse template parameters that are children nodes int parm_index =3D 0; - for (xmlNodePtr n =3D node->children; n; n =3D n->next) - { - if (n->type !=3D XML_ELEMENT_NODE) - continue; - - if (shared_ptr p =3D - build_template_parameter(ctxt, n, parm_index, result)) - { - result->add_template_parameter(p); - ++parm_index; - } - } + for (xmlNodePtr n =3D xmlFirstElementChild(node); + n; + n =3D xmlNextElementSibling(n)) + if (shared_ptr p =3D + build_template_parameter(ctxt, n, parm_index, result)) + { + result->add_template_parameter(p); + ++parm_index; + } =20 if (result) { --=20 2.30.0 --=20 Dodji