[PATCH] abg-reader: eliminate unreachable-code-loop-increment warnings

[PATCH] abg-reader: eliminate unreachable-code-loop-increment warnings

[Giuliano Procida]

This commit tidies up some debris from the change to use
xmlFirstElementChild and xmlNextElementSibling. A couple of while
loops had bodies which would be executed exactly once searching for
the next element with a given tag. These trigger warnings with recent
Clang++, at least.

This commit replaces the loops with their bodies and eliminates
a little redundant conditional logic.

There is no change in behaviour.

	* src/abg-reader.cc (read_translation_unit_from_input): In the
	branch where the context object holds a non-null node, just
	check that it an abi-instr; remove the final null check on
	node as it is guaranteed to be non-null.
	(read_elf_needed_from_input): Save the context object's node
	in node immediately; in the branch where the context object
	holds a non-null node, just check that it an elf-needed;
	remove the final null check on node as it is guaranteed to be
	non-null.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-reader.cc | 41 +++++++++--------------------------------
 1 file changed, 9 insertions(+), 32 deletions(-)

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 0b8149f6..793ca828 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -1576,21 +1576,10 @@ read_translation_unit_from_input(read_context&	ctxt)
     }
   else
     {
-      node = 0;
-      for (xmlNodePtr n = ctxt.get_corpus_node();
-	   n;
-	   n = xmlNextElementSibling(n))
-	{
-	  if (!xmlStrEqual(n->name, BAD_CAST("abi-instr")))
-	    return nil;
-	  node = n;
-	  break;
-	}
+      if (!xmlStrEqual(node->name, BAD_CAST("abi-instr")))
+	return nil;
     }
 
-  if (node == 0)
-    return nil;
-
   tu = get_or_read_and_add_translation_unit(ctxt, node);
 
   if (ctxt.get_corpus_node())
@@ -1740,9 +1729,8 @@ read_elf_needed_from_input(read_context&	ctxt,
   if (!reader)
     return false;
 
-  xmlNodePtr node = 0;
-
-  if (ctxt.get_corpus_node() == 0)
+  xmlNodePtr node = ctxt.get_corpus_node();
+  if (!node)
     {
       int status = 1;
       while (status == 1
@@ -1762,24 +1750,13 @@ read_elf_needed_from_input(read_context&	ctxt,
     }
   else
     {
-      for (xmlNodePtr n = ctxt.get_corpus_node();
-	   n;
-	   n = xmlNextElementSibling(n))
-	{
-	  if (!xmlStrEqual(n->name, BAD_CAST("elf-needed")))
-	    return false;
-	  node = n;
-	  break;
-	}
+      if (!xmlStrEqual(node->name, BAD_CAST("elf-needed")))
+        return false;
     }
 
-  bool result = false;
-  if (node)
-    {
-      result = build_needed(node, needed);
-      node = xmlNextElementSibling(node);
-      ctxt.set_corpus_node(node);
-    }
+  bool result = build_needed(node, needed);
+  node = xmlNextElementSibling(node);
+  ctxt.set_corpus_node(node);
 
   return result;
 }
-- 
2.32.0.272.g935e593368-goog

Re: [PATCH] abg-reader: eliminate unreachable-code-loop-increment warnings

[Matthias Maennich]

On Wed, Jun 16, 2021 at 02:30:03PM +0100, Giuliano Procida wrote:
>This commit tidies up some debris from the change to use
>xmlFirstElementChild and xmlNextElementSibling. A couple of while
>loops had bodies which would be executed exactly once searching for
>the next element with a given tag. These trigger warnings with recent
>Clang++, at least.
>
>This commit replaces the loops with their bodies and eliminates
>a little redundant conditional logic.
>
>There is no change in behaviour.
>
>	* src/abg-reader.cc (read_translation_unit_from_input): In the
>	branch where the context object holds a non-null node, just
>	check that it an abi-instr; remove the final null check on
>	node as it is guaranteed to be non-null.
>	(read_elf_needed_from_input): Save the context object's node
>	in node immediately; in the branch where the context object
>	holds a non-null node, just check that it an elf-needed;
>	remove the final null check on node as it is guaranteed to be
>	non-null.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> src/abg-reader.cc | 41 +++++++++--------------------------------
> 1 file changed, 9 insertions(+), 32 deletions(-)
>
>diff --git a/src/abg-reader.cc b/src/abg-reader.cc
>index 0b8149f6..793ca828 100644
>--- a/src/abg-reader.cc
>+++ b/src/abg-reader.cc
>@@ -1576,21 +1576,10 @@ read_translation_unit_from_input(read_context&	ctxt)
>     }
>   else
>     {
>-      node = 0;
>-      for (xmlNodePtr n = ctxt.get_corpus_node();
>-	   n;
>-	   n = xmlNextElementSibling(n))
>-	{
>-	  if (!xmlStrEqual(n->name, BAD_CAST("abi-instr")))
>-	    return nil;
>-	  node = n;
>-	  break;
>-	}
>+      if (!xmlStrEqual(node->name, BAD_CAST("abi-instr")))
>+	return nil;
>     }
>
>-  if (node == 0)
>-    return nil;
>-
>   tu = get_or_read_and_add_translation_unit(ctxt, node);
>
>   if (ctxt.get_corpus_node())
>@@ -1740,9 +1729,8 @@ read_elf_needed_from_input(read_context&	ctxt,
>   if (!reader)
>     return false;
>
>-  xmlNodePtr node = 0;
>-
>-  if (ctxt.get_corpus_node() == 0)
>+  xmlNodePtr node = ctxt.get_corpus_node();
>+  if (!node)
>     {
>       int status = 1;
>       while (status == 1
>@@ -1762,24 +1750,13 @@ read_elf_needed_from_input(read_context&	ctxt,
>     }
>   else
>     {
>-      for (xmlNodePtr n = ctxt.get_corpus_node();
>-	   n;
>-	   n = xmlNextElementSibling(n))
>-	{
>-	  if (!xmlStrEqual(n->name, BAD_CAST("elf-needed")))
>-	    return false;
>-	  node = n;
>-	  break;
>-	}
>+      if (!xmlStrEqual(node->name, BAD_CAST("elf-needed")))
>+        return false;
>     }
>
>-  bool result = false;
>-  if (node)
>-    {
>-      result = build_needed(node, needed);
>-      node = xmlNextElementSibling(node);
>-      ctxt.set_corpus_node(node);
>-    }
>+  bool result = build_needed(node, needed);
>+  node = xmlNextElementSibling(node);
>+  ctxt.set_corpus_node(node);
>
>   return result;
> }
>-- 
>2.32.0.272.g935e593368-goog
>