[PATCH] Fix dir command for duplicated paths

[PATCH] Fix dir command for duplicated paths

[Edjunior Barbosa Machado]

Hi,

noticed this issue with dir command for duplicated paths on upstream gdb:

$ ./gdb -q -ex "dir /tmp/a /tmp/b /tmp/c" -ex "dir /tmp/b /tmp/d /tmp/c"
Source directories searched: /tmp/a:/tmp/b:/tmp/c:$cdir:$cwd
Source directories searched: /tmp/b:/tmp/d:/tmp/c:/tmp/a:/tmp/b:/tmp/c:$cdir:$cwd

In gdb 7.4, the output used to be correct:

Source directories searched: /tmp/a:/tmp/b:/tmp/c:$cdir:$cwd
Source directories searched: /tmp/b:/tmp/d:/tmp/c:/tmp/a:$cdir:$cwd

This issue was introduced with a change on gdb/source.c from this patch:
https://sourceware.org/ml/gdb-patches/2012-04/msg00086.html

The patch below intends to fixes this regression using the original block of
code that checks for duplicated paths along the previously included paths.

Ok?

Thanks,
--
Edjunior


gdb/
2014-01-05  Edjunior Barbosa Machado  <emachado@linux.vnet.ibm.com>

	* source.c (add_path): Fix check for duplicated paths in the previously
	included paths.

---
 gdb/source.c |   32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/gdb/source.c b/gdb/source.c
index b75a7b4..4da5a95 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -574,17 +574,29 @@ add_path (char *dirname, char **which_path, int parse_separators)
 	char tinybuf[2];
 
 	p = *which_path;
-	/* FIXME: we should use realpath() or its work-alike
-	   before comparing.  Then all the code above which
-	   removes excess slashes and dots could simply go away.  */
-	if (!filename_cmp (p, name))
+	while (1)
 	  {
-	    /* Found it in the search path, remove old copy.  */
-	    if (p > *which_path)
-	      p--;		/* Back over leading separator.  */
-	    if (prefix > p - *which_path)
-	      goto skip_dup;	/* Same dir twice in one cmd.  */
-	    memmove (p, &p[len + 1], strlen (&p[len + 1]) + 1);	/* Copy from next \0 or  : */
+	    /* FIXME: we should use realpath() or its work-alike
+	       before comparing.  Then all the code above which
+	       removes excess slashes and dots could simply go away.  */
+	    if (!filename_ncmp (p, name, len)
+		&& (p[len] == '\0' || p[len] == DIRNAME_SEPARATOR))
+	      {
+		/* Found it in the search path, remove old copy.  */
+		if (p > *which_path)
+		  /* Back over leading separator.  */
+		  p--;
+		if (prefix > p - *which_path)
+		  /* Same dir twice in one cmd.  */
+		  goto skip_dup;
+		/* Copy from next '\0' or ':'.  */
+		memmove (p, &p[len + 1], strlen (&p[len + 1]) + 1);
+	      }
+	    p = strchr (p, DIRNAME_SEPARATOR);
+	    if (p != 0)
+	      ++p;
+	    else
+	      break;
 	  }
 
 	tinybuf[0] = DIRNAME_SEPARATOR;
-- 
1.7.9.5

Re: [PATCH] Fix dir command for duplicated paths

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 2276 bytes --]

On Mon, 06 Jan 2014 02:50:55 +0100, Edjunior Barbosa Machado wrote:
> The patch below intends to fixes this regression using the original block of
> code that checks for duplicated paths along the previously included paths.

OK with the patch, minor modification below and the new testcase.

Thanks for catching the regression.


Jan


[...]
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -574,17 +574,29 @@ add_path (char *dirname, char **which_path, int parse_separators)
>  	char tinybuf[2];
>  
>  	p = *which_path;
> -	/* FIXME: we should use realpath() or its work-alike
> -	   before comparing.  Then all the code above which
> -	   removes excess slashes and dots could simply go away.  */
> -	if (!filename_cmp (p, name))
> +	while (1)
>  	  {
> -	    /* Found it in the search path, remove old copy.  */
> -	    if (p > *which_path)
> -	      p--;		/* Back over leading separator.  */
> -	    if (prefix > p - *which_path)
> -	      goto skip_dup;	/* Same dir twice in one cmd.  */
> -	    memmove (p, &p[len + 1], strlen (&p[len + 1]) + 1);	/* Copy from next \0 or  : */
> +	    /* FIXME: we should use realpath() or its work-alike
> +	       before comparing.  Then all the code above which
> +	       removes excess slashes and dots could simply go away.  */
> +	    if (!filename_ncmp (p, name, len)
> +		&& (p[len] == '\0' || p[len] == DIRNAME_SEPARATOR))
> +	      {
> +		/* Found it in the search path, remove old copy.  */
> +		if (p > *which_path)


> +		  /* Back over leading separator.  */
> +		  p--;

		  {
		    /* Back over leading separator.  */
		    p--;
		  }
due to (sorry for the rule of mine):
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
Any two or more lines in code should be wrapped in braces, even if they are
comments, as they look like separate statements:


> +		if (prefix > p - *which_path)

> +		  /* Same dir twice in one cmd.  */
> +		  goto skip_dup;

		  {
		    /* Same dir twice in one cmd.  */
		    goto skip_dup;
		  }

> +		/* Copy from next '\0' or ':'.  */
> +		memmove (p, &p[len + 1], strlen (&p[len + 1]) + 1);
> +	      }
> +	    p = strchr (p, DIRNAME_SEPARATOR);
> +	    if (p != 0)
> +	      ++p;
> +	    else
> +	      break;
>  	  }
>  
>  	tinybuf[0] = DIRNAME_SEPARATOR;
> -- 
> 1.7.9.5

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 1318 bytes --]

gdb/testsuite/
2014-01-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/source-dir.exp: New file.

diff --git a/gdb/testsuite/gdb.base/source-dir.exp b/gdb/testsuite/gdb.base/source-dir.exp
new file mode 100644
index 0000000..43b56b7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/source-dir.exp
@@ -0,0 +1,23 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile
+
+gdb_start
+
+set foo "/nOtExStInG"
+
+gdb_test "directory $foo/a $foo/b $foo/c" "\r\nSource directories searched: $foo/a:$foo/b:$foo/c:\\\$cdir:\\\$cwd"
+gdb_test "directory $foo/b $foo/d $foo/c" "\r\nSource directories searched: $foo/b:$foo/d:$foo/c:$foo/a:\\\$cdir:\\\$cwd"

Re: [PATCH] Fix dir command for duplicated paths

[Edjunior Barbosa Machado]

On 01/06/2014 07:44 PM, Jan Kratochvil wrote:
> On Mon, 06 Jan 2014 02:50:55 +0100, Edjunior Barbosa Machado wrote:
>> The patch below intends to fixes this regression using the original block of
>> code that checks for duplicated paths along the previously included paths.
> 
> OK with the patch, minor modification below and the new testcase.
> 
> Thanks for catching the regression.
> 

Thanks Jan for the review and the new testcase. I've just checked in:

https://sourceware.org/ml/gdb-cvs/2014-01/msg00024.html

--
Edjunior