[PATCH] Fix file find utils and add unit tests (PR driver/81829).

[PATCH] Fix file find utils and add unit tests (PR driver/81829).

[Martin Liška]

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

Hi.

As shown in the PR, remove_prefix function is written wrongly. It does not distinguish
in between a node in linked list and pprefix->plist. So I decide to rewrite it.
Apart from that, I identified discrepancy in between do_add_prefix and prefix_from_string
where the later one appends always DIR_SEPARATOR (if missing). So I also the former function.
And I'm adding unit tests for all functions in file-find.c.

Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/ChangeLog:

2017-08-14  Martin Liska  <mliska@suse.cz>

	PR driver/81829
	* file-find.c (do_add_prefix): Always append DIR_SEPARATOR
	at the end of a prefix.
	(remove_prefix): Properly remove elements and accept also
	path without a trailing DIR_SEPARATOR.
	(purge): New function.
	(file_find_verify_prefix_creation): Likewise.
	(file_find_verify_prefix_add): Likewise.
	(file_find_verify_prefix_removal): Likewise.
	(file_find_c_tests): Likewise.
	* selftest-run-tests.c (selftest::run_tests): Add new
	file_find_c_tests.
	* selftest.h (file_find_c_tests): Likewise.
---
 gcc/file-find.c          | 182 ++++++++++++++++++++++++++++++++++++++++++-----
 gcc/selftest-run-tests.c |   1 +
 gcc/selftest.h           |   1 +
 3 files changed, 167 insertions(+), 17 deletions(-)



[-- Attachment #2: 0001-Fix-file-find-utils-and-add-unit-tests-PR-driver-818.patch --]
[-- Type: text/x-patch, Size: 6344 bytes --]

diff --git a/gcc/file-find.c b/gcc/file-find.c
index b072a4993d7..23a883042a2 100644
--- a/gcc/file-find.c
+++ b/gcc/file-find.c
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "filenames.h"
 #include "file-find.h"
+#include "selftest.h"
 
 static bool debug = false;
 
@@ -126,11 +127,22 @@ do_add_prefix (struct path_prefix *pprefix, const char *prefix, bool first)
   /* Keep track of the longest prefix.  */
 
   len = strlen (prefix);
+  bool append_separator = !IS_DIR_SEPARATOR (prefix[len - 1]);
+  if (append_separator)
+    len++;
+
   if (len > pprefix->max_len)
     pprefix->max_len = len;
 
   pl = XNEW (struct prefix_list);
-  pl->prefix = xstrdup (prefix);
+  char *dup = XCNEWVEC (char, len + 1);
+  memcpy (dup, prefix, append_separator ? len - 1 : len);
+  if (append_separator)
+    {
+      dup[len - 1] = DIR_SEPARATOR;
+      dup[len] = '\0';
+    }
+  pl->prefix = dup;
 
   if (*prev)
     pl->next = *prev;
@@ -212,34 +224,170 @@ prefix_from_string (const char *p, struct path_prefix *pprefix)
 void
 remove_prefix (const char *prefix, struct path_prefix *pprefix)
 {
-  struct prefix_list *remove, **prev, **remove_prev = NULL;
+  char *dup = NULL;
   int max_len = 0;
+  size_t len = strlen (prefix);
+  if (prefix[len - 1] != DIR_SEPARATOR)
+    {
+      char *dup = XNEWVEC (char, len + 2);
+      memcpy (dup, prefix, len);
+      dup[len] = DIR_SEPARATOR;
+      dup[len + 1] = '\0';
+      prefix = dup;
+    }
 
   if (pprefix->plist)
     {
-      prev = &pprefix->plist;
-      for (struct prefix_list *pl = pprefix->plist; pl->next; pl = pl->next)
+      prefix_list *prev = NULL;
+      for (struct prefix_list *pl = pprefix->plist; pl;)
 	{
 	  if (strcmp (prefix, pl->prefix) == 0)
 	    {
-	      remove = pl;
-	      remove_prev = prev;
-	      continue;
+	      if (prev == NULL)
+		pprefix->plist = pl->next;
+	      else
+		prev->next = pl->next;
+
+	      prefix_list *remove = pl;
+	      free (remove);
+	      pl = pl->next;
 	    }
+	  else
+	    {
+	      prev = pl;
 
-	  int l = strlen (pl->prefix);
-	  if (l > max_len)
-	    max_len = l;
-
-	  prev = &pl;
-	}
+	      int l = strlen (pl->prefix);
+	      if (l > max_len)
+		max_len = l;
 
-      if (remove_prev)
-	{
-	  *remove_prev = remove->next;
-	  free (remove);
+	      pl = pl->next;
+	    }
 	}
 
       pprefix->max_len = max_len;
     }
+
+  if (dup)
+    free (dup);
+}
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Encode '#' and '_' to path and dir separators in order to test portability
+   of the test-cases.  */
+
+static char *
+purge (const char *input)
+{
+  char *s = xstrdup (input);
+  for (char *c = s; *c != '\0'; c++)
+    switch (*c)
+      {
+      case '/':
+      case ':':
+	*c = 'a'; /* Poison default string values.  */
+	break;
+      case '_':
+	*c = PATH_SEPARATOR;
+	break;
+      case '#':
+	*c = DIR_SEPARATOR;
+	break;
+      default:
+	break;
+      }
+
+  return s;
+}
+
+const char *env1 = purge ("#home#user#bin_#home#user#bin_#bin_#usr#bin");
+const char *env2 = purge ("#root_#root_#root");
+
+/* Verify creation of prefix.  */
+
+static void
+file_find_verify_prefix_creation (void)
+{
+  path_prefix prefix;
+  memset (&prefix, 0, sizeof (prefix));
+  prefix_from_string (env1, &prefix);
+
+  ASSERT_EQ (15, prefix.max_len);
+
+  /* All prefixes end with DIR_SEPARATOR.  */
+  ASSERT_STREQ (purge ("#home#user#bin#"), prefix.plist->prefix);
+  ASSERT_STREQ (purge ("#home#user#bin#"), prefix.plist->next->prefix);
+  ASSERT_STREQ (purge ("#bin#"), prefix.plist->next->next->prefix);
+  ASSERT_STREQ (purge ("#usr#bin#"), prefix.plist->next->next->next->prefix);
+  ASSERT_EQ (NULL, prefix.plist->next->next->next->next);
+}
+
+/* Verify adding a prefix.  */
+
+static void
+file_find_verify_prefix_add (void)
+{
+  path_prefix prefix;
+  memset (&prefix, 0, sizeof (prefix));
+  prefix_from_string (env1, &prefix);
+
+  add_prefix (&prefix, purge ("#root"));
+  ASSERT_STREQ (purge ("#home#user#bin#"), prefix.plist->prefix);
+  ASSERT_STREQ (purge ("#root#"),
+		prefix.plist->next->next->next->next->prefix);
+
+  add_prefix_begin (&prefix, purge ("#var"));
+  ASSERT_STREQ (purge ("#var#"), prefix.plist->prefix);
+}
+
+/* Verify adding a prefix.  */
+
+static void
+file_find_verify_prefix_removal (void)
+{
+  path_prefix prefix;
+  memset (&prefix, 0, sizeof (prefix));
+  prefix_from_string (env1, &prefix);
+
+  /* All occurences of a prefix should be removed.  */
+  remove_prefix (purge ("#home#user#bin"), &prefix);
+
+  ASSERT_EQ (9, prefix.max_len);
+  ASSERT_STREQ (purge ("#bin#"), prefix.plist->prefix);
+  ASSERT_STREQ (purge ("#usr#bin#"), prefix.plist->next->prefix);
+  ASSERT_EQ (NULL, prefix.plist->next->next);
+
+  remove_prefix (purge ("#usr#bin#"), &prefix);
+  ASSERT_EQ (5, prefix.max_len);
+  ASSERT_STREQ (purge ("#bin#"), prefix.plist->prefix);
+  ASSERT_EQ (NULL, prefix.plist->next);
+
+  remove_prefix (purge ("#dev#random#"), &prefix);
+  remove_prefix (purge ("#bi#"), &prefix);
+
+  remove_prefix (purge ("#bin#"), &prefix);
+  ASSERT_EQ (NULL, prefix.plist);
+  ASSERT_EQ (0, prefix.max_len);
+
+  memset (&prefix, 0, sizeof (prefix));
+  prefix_from_string (env2, &prefix);
+  ASSERT_EQ (6, prefix.max_len);
+
+  remove_prefix (purge ("#root#"), &prefix);
+  ASSERT_EQ (NULL, prefix.plist);
+  ASSERT_EQ (0, prefix.max_len);
+}
+
+/* Run all of the selftests within this file.  */
+
+void file_find_c_tests ()
+{
+  file_find_verify_prefix_creation ();
+  file_find_verify_prefix_add ();
+  file_find_verify_prefix_removal ();
 }
+
+} // namespace selftest
+#endif /* CHECKING_P */
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 30e476d14c5..dadc71a7c2a 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -66,6 +66,7 @@ selftest::run_tests ()
   sreal_c_tests ();
   fibonacci_heap_c_tests ();
   typed_splay_tree_c_tests ();
+  file_find_c_tests ();
 
   /* Mid-level data structures.  */
   input_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 0572fefd281..cfa3a82bc60 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -197,6 +197,7 @@ extern void tree_cfg_c_tests ();
 extern void vec_c_tests ();
 extern void wide_int_cc_tests ();
 extern void predict_c_tests ();
+extern void file_find_c_tests ();
 
 extern int num_passes;
 

Re: [PATCH] Fix file find utils and add unit tests (PR driver/81829).

[Martin Liška]

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

On 08/15/2017 02:45 PM, Martin Liška wrote:
> Hi.
> 
> As shown in the PR, remove_prefix function is written wrongly. It does not distinguish
> in between a node in linked list and pprefix->plist. So I decide to rewrite it.
> Apart from that, I identified discrepancy in between do_add_prefix and prefix_from_string
> where the later one appends always DIR_SEPARATOR (if missing). So I also the former function.
> And I'm adding unit tests for all functions in file-find.c.
> 
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin
> 
> gcc/ChangeLog:
> 
> 2017-08-14  Martin Liska  <mliska@suse.cz>
> 
> 	PR driver/81829
> 	* file-find.c (do_add_prefix): Always append DIR_SEPARATOR
> 	at the end of a prefix.
> 	(remove_prefix): Properly remove elements and accept also
> 	path without a trailing DIR_SEPARATOR.
> 	(purge): New function.
> 	(file_find_verify_prefix_creation): Likewise.
> 	(file_find_verify_prefix_add): Likewise.
> 	(file_find_verify_prefix_removal): Likewise.
> 	(file_find_c_tests): Likewise.
> 	* selftest-run-tests.c (selftest::run_tests): Add new
> 	file_find_c_tests.
> 	* selftest.h (file_find_c_tests): Likewise.
> ---
>  gcc/file-find.c          | 182 ++++++++++++++++++++++++++++++++++++++++++-----
>  gcc/selftest-run-tests.c |   1 +
>  gcc/selftest.h           |   1 +
>  3 files changed, 167 insertions(+), 17 deletions(-)
> 
> 

Hi.

As doko pointed out, the first version was not correct. Let me describe 2 scenarios which should
be supported:

1) my original motivation where I configure gcc w/ --prefix=/my_bin and I manually create in /my_bin/bin:

$ ln -s gcc-ar ar
$ ln -s gcc-nm nm
$ ln -s gcc-ranlib ranlib

and then is set PATH=/my_bin/bin:... and LD_LIBRARY_PATH and I don't have to care about NM=gcc-nm and so on.
That's how I usually test a newly built GCC.

2) using NM=gcc-nm and so on will force usage of LTO plugin. That's what was broken in first version of the patch.

In order to support both cases we need to identify real name of GCC wrapper (like /my_bin/bin/gcc-ar) and when
find_a_file (&path, real_exe_name, X_OK) will point to the same file, the directory should be ignored from prefix.

Can you please test attached patch in your scenario?
Thanks,
Martin

[-- Attachment #2: 0001-Fix-file-find-utils-and-add-unit-tests-PR-driver-818.patch --]
[-- Type: text/x-patch, Size: 8657 bytes --]

From f8029ed6d3dd444ee2608146118f2189cf9ef0d8 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Mon, 14 Aug 2017 13:56:32 +0200
Subject: [PATCH] Fix file find utils and add unit tests (PR driver/81829).

gcc/ChangeLog:

2017-08-14  Martin Liska  <mliska@suse.cz>

	PR driver/81829
	* file-find.c (do_add_prefix): Always append DIR_SEPARATOR
	at the end of a prefix.
	(remove_prefix): Properly remove elements and accept also
	path without a trailing DIR_SEPARATOR.
	(purge): New function.
	(file_find_verify_prefix_creation): Likewise.
	(file_find_verify_prefix_add): Likewise.
	(file_find_verify_prefix_removal): Likewise.
	(file_find_c_tests): Likewise.
	* selftest-run-tests.c (selftest::run_tests): Add new
	file_find_c_tests.
	* selftest.h (file_find_c_tests): Likewise.
---
 gcc/file-find.c          | 182 ++++++++++++++++++++++++++++++++++++++++++-----
 gcc/gcc-ar.c             |  19 +++--
 gcc/selftest-run-tests.c |   1 +
 gcc/selftest.h           |   1 +
 4 files changed, 179 insertions(+), 24 deletions(-)

diff --git a/gcc/file-find.c b/gcc/file-find.c
index b072a4993d7..23a883042a2 100644
--- a/gcc/file-find.c
+++ b/gcc/file-find.c
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "system.h"
 #include "filenames.h"
 #include "file-find.h"
+#include "selftest.h"
 
 static bool debug = false;
 
@@ -126,11 +127,22 @@ do_add_prefix (struct path_prefix *pprefix, const char *prefix, bool first)
   /* Keep track of the longest prefix.  */
 
   len = strlen (prefix);
+  bool append_separator = !IS_DIR_SEPARATOR (prefix[len - 1]);
+  if (append_separator)
+    len++;
+
   if (len > pprefix->max_len)
     pprefix->max_len = len;
 
   pl = XNEW (struct prefix_list);
-  pl->prefix = xstrdup (prefix);
+  char *dup = XCNEWVEC (char, len + 1);
+  memcpy (dup, prefix, append_separator ? len - 1 : len);
+  if (append_separator)
+    {
+      dup[len - 1] = DIR_SEPARATOR;
+      dup[len] = '\0';
+    }
+  pl->prefix = dup;
 
   if (*prev)
     pl->next = *prev;
@@ -212,34 +224,170 @@ prefix_from_string (const char *p, struct path_prefix *pprefix)
 void
 remove_prefix (const char *prefix, struct path_prefix *pprefix)
 {
-  struct prefix_list *remove, **prev, **remove_prev = NULL;
+  char *dup = NULL;
   int max_len = 0;
+  size_t len = strlen (prefix);
+  if (prefix[len - 1] != DIR_SEPARATOR)
+    {
+      char *dup = XNEWVEC (char, len + 2);
+      memcpy (dup, prefix, len);
+      dup[len] = DIR_SEPARATOR;
+      dup[len + 1] = '\0';
+      prefix = dup;
+    }
 
   if (pprefix->plist)
     {
-      prev = &pprefix->plist;
-      for (struct prefix_list *pl = pprefix->plist; pl->next; pl = pl->next)
+      prefix_list *prev = NULL;
+      for (struct prefix_list *pl = pprefix->plist; pl;)
 	{
 	  if (strcmp (prefix, pl->prefix) == 0)
 	    {
-	      remove = pl;
-	      remove_prev = prev;
-	      continue;
+	      if (prev == NULL)
+		pprefix->plist = pl->next;
+	      else
+		prev->next = pl->next;
+
+	      prefix_list *remove = pl;
+	      free (remove);
+	      pl = pl->next;
 	    }
+	  else
+	    {
+	      prev = pl;
 
-	  int l = strlen (pl->prefix);
-	  if (l > max_len)
-	    max_len = l;
-
-	  prev = &pl;
-	}
+	      int l = strlen (pl->prefix);
+	      if (l > max_len)
+		max_len = l;
 
-      if (remove_prev)
-	{
-	  *remove_prev = remove->next;
-	  free (remove);
+	      pl = pl->next;
+	    }
 	}
 
       pprefix->max_len = max_len;
     }
+
+  if (dup)
+    free (dup);
+}
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Encode '#' and '_' to path and dir separators in order to test portability
+   of the test-cases.  */
+
+static char *
+purge (const char *input)
+{
+  char *s = xstrdup (input);
+  for (char *c = s; *c != '\0'; c++)
+    switch (*c)
+      {
+      case '/':
+      case ':':
+	*c = 'a'; /* Poison default string values.  */
+	break;
+      case '_':
+	*c = PATH_SEPARATOR;
+	break;
+      case '#':
+	*c = DIR_SEPARATOR;
+	break;
+      default:
+	break;
+      }
+
+  return s;
+}
+
+const char *env1 = purge ("#home#user#bin_#home#user#bin_#bin_#usr#bin");
+const char *env2 = purge ("#root_#root_#root");
+
+/* Verify creation of prefix.  */
+
+static void
+file_find_verify_prefix_creation (void)
+{
+  path_prefix prefix;
+  memset (&prefix, 0, sizeof (prefix));
+  prefix_from_string (env1, &prefix);
+
+  ASSERT_EQ (15, prefix.max_len);
+
+  /* All prefixes end with DIR_SEPARATOR.  */
+  ASSERT_STREQ (purge ("#home#user#bin#"), prefix.plist->prefix);
+  ASSERT_STREQ (purge ("#home#user#bin#"), prefix.plist->next->prefix);
+  ASSERT_STREQ (purge ("#bin#"), prefix.plist->next->next->prefix);
+  ASSERT_STREQ (purge ("#usr#bin#"), prefix.plist->next->next->next->prefix);
+  ASSERT_EQ (NULL, prefix.plist->next->next->next->next);
+}
+
+/* Verify adding a prefix.  */
+
+static void
+file_find_verify_prefix_add (void)
+{
+  path_prefix prefix;
+  memset (&prefix, 0, sizeof (prefix));
+  prefix_from_string (env1, &prefix);
+
+  add_prefix (&prefix, purge ("#root"));
+  ASSERT_STREQ (purge ("#home#user#bin#"), prefix.plist->prefix);
+  ASSERT_STREQ (purge ("#root#"),
+		prefix.plist->next->next->next->next->prefix);
+
+  add_prefix_begin (&prefix, purge ("#var"));
+  ASSERT_STREQ (purge ("#var#"), prefix.plist->prefix);
+}
+
+/* Verify adding a prefix.  */
+
+static void
+file_find_verify_prefix_removal (void)
+{
+  path_prefix prefix;
+  memset (&prefix, 0, sizeof (prefix));
+  prefix_from_string (env1, &prefix);
+
+  /* All occurences of a prefix should be removed.  */
+  remove_prefix (purge ("#home#user#bin"), &prefix);
+
+  ASSERT_EQ (9, prefix.max_len);
+  ASSERT_STREQ (purge ("#bin#"), prefix.plist->prefix);
+  ASSERT_STREQ (purge ("#usr#bin#"), prefix.plist->next->prefix);
+  ASSERT_EQ (NULL, prefix.plist->next->next);
+
+  remove_prefix (purge ("#usr#bin#"), &prefix);
+  ASSERT_EQ (5, prefix.max_len);
+  ASSERT_STREQ (purge ("#bin#"), prefix.plist->prefix);
+  ASSERT_EQ (NULL, prefix.plist->next);
+
+  remove_prefix (purge ("#dev#random#"), &prefix);
+  remove_prefix (purge ("#bi#"), &prefix);
+
+  remove_prefix (purge ("#bin#"), &prefix);
+  ASSERT_EQ (NULL, prefix.plist);
+  ASSERT_EQ (0, prefix.max_len);
+
+  memset (&prefix, 0, sizeof (prefix));
+  prefix_from_string (env2, &prefix);
+  ASSERT_EQ (6, prefix.max_len);
+
+  remove_prefix (purge ("#root#"), &prefix);
+  ASSERT_EQ (NULL, prefix.plist);
+  ASSERT_EQ (0, prefix.max_len);
+}
+
+/* Run all of the selftests within this file.  */
+
+void file_find_c_tests ()
+{
+  file_find_verify_prefix_creation ();
+  file_find_verify_prefix_add ();
+  file_find_verify_prefix_removal ();
 }
+
+} // namespace selftest
+#endif /* CHECKING_P */
diff --git a/gcc/gcc-ar.c b/gcc/gcc-ar.c
index 78d2fc1ad30..1155ba83e35 100644
--- a/gcc/gcc-ar.c
+++ b/gcc/gcc-ar.c
@@ -194,15 +194,20 @@ main (int ac, char **av)
 #ifdef CROSS_DIRECTORY_STRUCTURE
       real_exe_name = concat (target_machine, "-", PERSONALITY, NULL);
 #endif
-      /* Do not search original location in the same folder.  */
-      char *exe_folder = lrealpath (av[0]);
-      exe_folder[strlen (exe_folder) - strlen (lbasename (exe_folder))] = '\0';
-      char *location = concat (exe_folder, PERSONALITY, NULL);
+      char *wrapper_file = lrealpath (av[0]);
+      exe_name = lrealpath (find_a_file (&path, real_exe_name, X_OK));
 
-      if (access (location, X_OK) == 0)
-	remove_prefix (exe_folder, &path);
+      /* If the exe_name points to the wrapper, remove folder of the wrapper
+	 from prefix and try search again.  */
+      if (strcmp (exe_name, wrapper_file) == 0)
+	{
+	  char *exe_folder = wrapper_file;
+	  exe_folder[strlen (exe_folder) - strlen (lbasename (exe_folder))] = '\0';
+	  remove_prefix (exe_folder, &path);
+
+	  exe_name = find_a_file (&path, real_exe_name, X_OK);
+	}
 
-      exe_name = find_a_file (&path, real_exe_name, X_OK);
       if (!exe_name)
 	{
 	  fprintf (stderr, "%s: Cannot find binary '%s'\n", av[0],
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 30e476d14c5..dadc71a7c2a 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -66,6 +66,7 @@ selftest::run_tests ()
   sreal_c_tests ();
   fibonacci_heap_c_tests ();
   typed_splay_tree_c_tests ();
+  file_find_c_tests ();
 
   /* Mid-level data structures.  */
   input_c_tests ();
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 0572fefd281..cfa3a82bc60 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -197,6 +197,7 @@ extern void tree_cfg_c_tests ();
 extern void vec_c_tests ();
 extern void wide_int_cc_tests ();
 extern void predict_c_tests ();
+extern void file_find_c_tests ();
 
 extern int num_passes;
 
-- 
2.13.3

Re: [PATCH] Fix file find utils and add unit tests (PR driver/81829).

[Martin Sebor]

On 08/18/2017 04:17 AM, Martin Liška wrote:
> On 08/15/2017 02:45 PM, Martin Liška wrote:
>> Hi.
>>
>> As shown in the PR, remove_prefix function is written wrongly. It does not distinguish
>> in between a node in linked list and pprefix->plist. So I decide to rewrite it.
>> Apart from that, I identified discrepancy in between do_add_prefix and prefix_from_string
>> where the later one appends always DIR_SEPARATOR (if missing). So I also the former function.
>> And I'm adding unit tests for all functions in file-find.c.

I know only very little about this API but from a quick glance at
the change it looks to me like it introduces an implicit assumption
that prefix points to a non-empty string.  If that is in fact one
of the function's preconditions I would suggest to a) assert it
before relying on it, and b) document it.  Otherwise, if the prefix
is allowed to be empty then the code below is undefined in that
case.

Martin

@@ -126,11 +127,22 @@ do_add_prefix (struct path_prefix *pprefix, const 
char *prefix, bool first)
    /* Keep track of the longest prefix.  */

    len = strlen (prefix);
+  bool append_separator = !IS_DIR_SEPARATOR (prefix[len - 1]);
+  if (append_separator)
+    len++;
+
    if (len > pprefix->max_le

Re: [PATCH] Fix file find utils and add unit tests (PR driver/81829).

[Martin Liška]

On 08/25/2017 06:41 PM, Martin Sebor wrote:
> On 08/18/2017 04:17 AM, Martin Liška wrote:
>> On 08/15/2017 02:45 PM, Martin Liška wrote:
>>> Hi.
>>>
>>> As shown in the PR, remove_prefix function is written wrongly. It does not distinguish
>>> in between a node in linked list and pprefix->plist. So I decide to rewrite it.
>>> Apart from that, I identified discrepancy in between do_add_prefix and prefix_from_string
>>> where the later one appends always DIR_SEPARATOR (if missing). So I also the former function.
>>> And I'm adding unit tests for all functions in file-find.c.
> 
> I know only very little about this API but from a quick glance at
> the change it looks to me like it introduces an implicit assumption
> that prefix points to a non-empty string.  If that is in fact one
> of the function's preconditions I would suggest to a) assert it
> before relying on it, and b) document it.  Otherwise, if the prefix
> is allowed to be empty then the code below is undefined in that
> case.
> 
> Martin
> 
> @@ -126,11 +127,22 @@ do_add_prefix (struct path_prefix *pprefix, const char *prefix, bool first)
>    /* Keep track of the longest prefix.  */
> 
>    len = strlen (prefix);
> +  bool append_separator = !IS_DIR_SEPARATOR (prefix[len - 1]);
> +  if (append_separator)
> +    len++;
> +
>    if (len > pprefix->max_le

Thanks Martin.

I'm tending to simplify the functionality, let's see what will be discussed in the PR.

Martin

[PATCH] Revert r238089 (PR driver/81829).

[Martin Liška]

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

Hi.

In order to make the code simple and transparent, I suggest to revert r238089.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

[-- Attachment #2: 0001-Revert-r238089-PR-driver-81829.patch --]
[-- Type: text/x-patch, Size: 2751 bytes --]

From 0a865d51a5f61d0fa13e5d4eea208c62ff89e32e Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Fri, 15 Sep 2017 14:02:13 +0200
Subject: [PATCH] Revert r238089 (PR driver/81829).

gcc/ChangeLog:

2017-09-15  Martin Liska  <mliska@suse.cz>

	PR driver/81829
	* file-find.c (remove_prefix): Remove.
	* file-find.h (remove_prefix): Likewise.
	* gcc-ar.c: Remove smartness of lookup.
---
 gcc/file-find.c | 35 -----------------------------------
 gcc/file-find.h |  1 -
 gcc/gcc-ar.c    |  8 --------
 3 files changed, 44 deletions(-)

diff --git a/gcc/file-find.c b/gcc/file-find.c
index b072a4993d7..b5a1fe8494e 100644
--- a/gcc/file-find.c
+++ b/gcc/file-find.c
@@ -208,38 +208,3 @@ prefix_from_string (const char *p, struct path_prefix *pprefix)
     }
   free (nstore);
 }
-
-void
-remove_prefix (const char *prefix, struct path_prefix *pprefix)
-{
-  struct prefix_list *remove, **prev, **remove_prev = NULL;
-  int max_len = 0;
-
-  if (pprefix->plist)
-    {
-      prev = &pprefix->plist;
-      for (struct prefix_list *pl = pprefix->plist; pl->next; pl = pl->next)
-	{
-	  if (strcmp (prefix, pl->prefix) == 0)
-	    {
-	      remove = pl;
-	      remove_prev = prev;
-	      continue;
-	    }
-
-	  int l = strlen (pl->prefix);
-	  if (l > max_len)
-	    max_len = l;
-
-	  prev = &pl;
-	}
-
-      if (remove_prev)
-	{
-	  *remove_prev = remove->next;
-	  free (remove);
-	}
-
-      pprefix->max_len = max_len;
-    }
-}
diff --git a/gcc/file-find.h b/gcc/file-find.h
index 8f49a3af273..407feba26e7 100644
--- a/gcc/file-find.h
+++ b/gcc/file-find.h
@@ -41,7 +41,6 @@ extern void find_file_set_debug (bool);
 extern char *find_a_file (struct path_prefix *, const char *, int);
 extern void add_prefix (struct path_prefix *, const char *);
 extern void add_prefix_begin (struct path_prefix *, const char *);
-extern void remove_prefix (const char *prefix, struct path_prefix *);
 extern void prefix_from_env (const char *, struct path_prefix *);
 extern void prefix_from_string (const char *, struct path_prefix *);
 
diff --git a/gcc/gcc-ar.c b/gcc/gcc-ar.c
index 78d2fc1ad30..d5d80e042e5 100644
--- a/gcc/gcc-ar.c
+++ b/gcc/gcc-ar.c
@@ -194,14 +194,6 @@ main (int ac, char **av)
 #ifdef CROSS_DIRECTORY_STRUCTURE
       real_exe_name = concat (target_machine, "-", PERSONALITY, NULL);
 #endif
-      /* Do not search original location in the same folder.  */
-      char *exe_folder = lrealpath (av[0]);
-      exe_folder[strlen (exe_folder) - strlen (lbasename (exe_folder))] = '\0';
-      char *location = concat (exe_folder, PERSONALITY, NULL);
-
-      if (access (location, X_OK) == 0)
-	remove_prefix (exe_folder, &path);
-
       exe_name = find_a_file (&path, real_exe_name, X_OK);
       if (!exe_name)
 	{
-- 
2.14.1

Re: [PATCH] Revert r238089 (PR driver/81829).

[Martin Liška]

PING^1

On 09/15/2017 02:07 PM, Martin Liška wrote:
> Hi.
> 
> In order to make the code simple and transparent, I suggest to revert r238089.
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> 
> Ready to be installed?
> Martin
>