[PATCH] Make decl_names_equal more accurate

[Dodji Seketeli]

Giuliano Procida <gprocida@google.com> a écrit:

> This utility function compares qualified names component-wise with
> special handling of components that are anonymous names.
>
> The code is incorrect for some cases when there are different numbers
> of components on each side. Also, the control flow in the loop body is
> more complex than it needs to be.

I am not sure the result is radically simpler.  But I agree it fixes the
problem.

Just for the record, I believe this much shorter change also fixes the
issue:

diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
index dfbec87..919b50f 100644
--- a/src/abg-tools-utils.cc
+++ b/src/abg-tools-utils.cc
@@ -540,7 +540,7 @@ decl_names_equal(const string& l, const string& r)
 			ANONYMOUS_ENUM_INTERNAL_NAME) == 0))
 	{
 	  if (l_pos2 == l.npos || r_pos2 == r.npos)
-	    return true;
+	    return (l_pos2 == l.npos) ==  (r_pos2 == r.npos);
 
 	  l_pos1 = l_pos2 + 2;
 	  r_pos1 = r_pos2 + 2;

But as I don't have any strong opinion either way, I am going with your
patch.


> This commit simplifies the control flow, fixes the comparison bugs and
> adds some extra tests to cover these cases.
>
> 	* src/abg-tools-utils.cc (decl_names_equal): Move {l,r}_pos2
> 	declarations into the loop and make {l,r}_length const. Avoid
> 	chance of arithmetic on string::npos values. Rework
> 	logic so there is a single test for "names compare equal" and
> 	a single test for different numbers of name components.
> 	* tests/test-tools-utils.cc (main): Add nine more tests.
>
> Signed-off-by: Giuliano Procida <gprocida@google.com>

Applied to master, thanks!

Cheers,

-- 
		Dodji