Merge forward-search/reverse-search, use gdb::def_vector, remove limit

Back in:

 commit 85ae1317ad
 Author:     Stan Shebs <shebs@codesourcery.com>
 AuthorDate: Thu Dec 8 02:27:47 1994 +0000

	     * source.c: Various cosmetic changes.
	     (forward_search_command): Handle very long source lines correctly.

a buffer with a hard limit was converted to a heap buffer:

  @@ -1228,15 +1284,26 @@ forward_search_command (regex, from_tty)
     stream = fdopen (desc, FOPEN_RT);
     clearerr (stream);
     while (1) {
  -/* FIXME!!!  We walk right off the end of buf if we get a long line!!! */
  -    char buf[4096];            /* Should be reasonable??? */
  -    register char *p = buf;
  +    static char *buf = NULL;
  +    register char *p;
  +    int cursize, newsize;
  +
  +    cursize = 256;
  +    buf = xmalloc (cursize);
  +    p = buf;

However, reverse_search_command has the exact same problem, and that
wasn't fixed.  We still have that "we walk right off" comment...

Recently, the xmalloc above was replaced with a xrealloc, because as
can be seen above, that 'buf' variable above was a static local,
otherwise we'd be leaking.  This commit replaces that and the
associated manual buffer growing with a gdb::def_vector<char>.  I
don't think there's much point in reusing the buffer across command
invocations.

While doing this, I realized that reverse_search_command is almost
identical to forward_search_command.  So this commit factors out a
common helper function instead of duplicating a lot of code.

There are some tests for "forward-search" in gdb.base/list.exp, but
since they use the "search" alias, they were a bit harder to find than
expected.  That's now fixed, both by testing both variants, and by
adding some commentary.  Also, there are no tests for the
"reverse-search" command, so this commit adds some for that too.

gdb/ChangeLog:
2018-12-08  Pedro Alves  <palves@redhat.com>

	* source.c (forward_search_command): Rename to ...
	(search_command_helper): ... this.  Add 'forward' parameter.
	Tweak to use a gdb::def_vector<char> instead of a xrealloc'ed
	buffer.  Handle backward searches too.
	(forward_search_command, reverse_search_command): Reimplement by
	calling search_command_helper.

gdb/testsuite/ChangeLog:
2018-12-08  Pedro Alves  <palves@redhat.com>

	* gdb.base/list.exp (test_forward_search): Rename to ...
	(test_forward_reverse_search): ... this.  Also test reverse-search
	and the forward-search alias.

gdb/ChangeLog

@@ -1,3 +1,12 @@
+2018-12-08  Pedro Alves  <palves@redhat.com>
+
+	* source.c (forward_search_command): Rename to ...
+	(search_command_helper): ... this.  Add 'forward' parameter.
+	Tweak to use a gdb::def_vector<char> instead of a xrealloc'ed
+	buffer.  Handle backward searches too.
+	(forward_search_command, reverse_search_command): Reimplement by
+	calling search_command_helper.
+
 2018-12-07  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* .dir-locals.el: Copy most of the settings from c-mode over to

gdb/source.c

@@ -1521,16 +1521,14 @@ info_line_command (const char *arg, int from_tty)
 
 /* Commands to search the source file for a regexp.  */
 
+/* Helper for forward_search_command/reverse_search_command.  FORWARD
+   indicates direction: true for forward, false for
+   backward/reverse.  */
+
 static void
-forward_search_command (const char *regex, int from_tty)
+search_command_helper (const char *regex, int from_tty, bool forward)
 {
-  int c;
+  const char *msg = re_comp (regex);
-  int line;
-  char *msg;
-
-  line = last_line_listed + 1;
-
-  msg = (char *) re_comp (regex);
   if (msg)
     error (("%s"), msg);
 
@@ -1544,6 +1542,10 @@ forward_search_command (const char *regex, int from_tty)
   if (current_source_symtab->line_charpos == 0)
     find_source_lines (current_source_symtab, desc.get ());
 
+  int line = (forward
+	      ? last_line_listed + 1
+	      : last_line_listed - 1);
+
   if (line < 1 || line > current_source_symtab->nlines)
     error (_("Expression not found"));
 
@@ -1553,43 +1555,35 @@ forward_search_command (const char *regex, int from_tty)
 
   gdb_file_up stream = desc.to_file (FDOPEN_MODE);
   clearerr (stream.get ());
+
+  gdb::def_vector<char> buf;
+  buf.reserve (256);
+
   while (1)
     {
-      static char *buf = NULL;
+      buf.resize (0);
-      char *p;
-      int cursize, newsize;
 
-      cursize = 256;
+      int c = fgetc (stream.get ());
-      buf = (char *) xrealloc (buf, cursize);
-      p = buf;
-
-      c = fgetc (stream.get ());
       if (c == EOF)
 	break;
       do
 	{
-	  *p++ = c;
+	  buf.push_back (c);
-	  if (p - buf == cursize)
-	    {
-	      newsize = cursize + cursize / 2;
-	      buf = (char *) xrealloc (buf, newsize);
-	      p = buf + cursize;
-	      cursize = newsize;
-	    }
 	}
       while (c != '\n' && (c = fgetc (stream.get ())) >= 0);
 
       /* Remove the \r, if any, at the end of the line, otherwise
          regular expressions that end with $ or \n won't work.  */
-      if (p - buf > 1 && p[-2] == '\r')
+      size_t sz = buf.size ();
+      if (sz >= 2 && buf[sz - 2] == '\r')
 	{
-	  p--;
+	  buf[sz - 2] = '\n';
-	  p[-1] = '\n';
+	  buf.resize (sz - 1);
 	}
 
       /* We now have a source line in buf, null terminate and match.  */
-      *p = 0;
+      buf.push_back ('\0');
-      if (re_exec (buf) > 0)
+      if (re_exec (buf.data ()) > 0)
 	{
 	  /* Match!  */
 	  print_source_lines (current_source_symtab, line, line + 1, 0);
@@ -1597,90 +1591,35 @@ forward_search_command (const char *regex, int from_tty)
 	  current_source_line = std::max (line - lines_to_list / 2, 1);
 	  return;
 	}
-      line++;
+
+      if (forward)
+	line++;
+      else
+	{
+	  line--;
+	  if (fseek (stream.get (),
+		     current_source_symtab->line_charpos[line - 1], 0) < 0)
+	    {
+	      const char *filename
+		= symtab_to_filename_for_display (current_source_symtab);
+	      perror_with_name (filename);
+	    }
+	}
     }
 
   printf_filtered (_("Expression not found\n"));
 }
 
+static void
+forward_search_command (const char *regex, int from_tty)
+{
+  search_command_helper (regex, from_tty, true);
+}
+
 static void
 reverse_search_command (const char *regex, int from_tty)
 {
-  int c;
+  search_command_helper (regex, from_tty, false);
-  int line;
-  char *msg;
-
-  line = last_line_listed - 1;
-
-  msg = (char *) re_comp (regex);
-  if (msg)
-    error (("%s"), msg);
-
-  if (current_source_symtab == 0)
-    select_source_symtab (0);
-
-  scoped_fd desc = open_source_file (current_source_symtab);
-  if (desc.get () < 0)
-    perror_with_name (symtab_to_filename_for_display (current_source_symtab));
-
-  if (current_source_symtab->line_charpos == 0)
-    find_source_lines (current_source_symtab, desc.get ());
-
-  if (line < 1 || line > current_source_symtab->nlines)
-    error (_("Expression not found"));
-
-  if (lseek (desc.get (), current_source_symtab->line_charpos[line - 1], 0)
-      < 0)
-    perror_with_name (symtab_to_filename_for_display (current_source_symtab));
-
-  gdb_file_up stream = desc.to_file (FDOPEN_MODE);
-  clearerr (stream.get ());
-  while (line > 1)
-    {
-/* FIXME!!!  We walk right off the end of buf if we get a long line!!!  */
-      char buf[4096];		/* Should be reasonable???  */
-      char *p = buf;
-
-      c = fgetc (stream.get ());
-      if (c == EOF)
-	break;
-      do
-	{
-	  *p++ = c;
-	}
-      while (c != '\n' && (c = fgetc (stream.get ())) >= 0);
-
-      /* Remove the \r, if any, at the end of the line, otherwise
-         regular expressions that end with $ or \n won't work.  */
-      if (p - buf > 1 && p[-2] == '\r')
-	{
-	  p--;
-	  p[-1] = '\n';
-	}
-
-      /* We now have a source line in buf; null terminate and match.  */
-      *p = 0;
-      if (re_exec (buf) > 0)
-	{
-	  /* Match!  */
-	  print_source_lines (current_source_symtab, line, line + 1, 0);
-	  set_internalvar_integer (lookup_internalvar ("_"), line);
-	  current_source_line = std::max (line - lines_to_list / 2, 1);
-	  return;
-	}
-      line--;
-      if (fseek (stream.get (),
-		 current_source_symtab->line_charpos[line - 1], 0) < 0)
-	{
-	  const char *filename;
-
-	  filename = symtab_to_filename_for_display (current_source_symtab);
-	  perror_with_name (filename);
-	}
-    }
-
-  printf_filtered (_("Expression not found\n"));
-  return;
 }
 
 /* If the last character of PATH is a directory separator, then strip it.  */

gdb/testsuite/ChangeLog

@@ -1,3 +1,9 @@
+2018-12-08  Pedro Alves  <palves@redhat.com>
+
+	* gdb.base/list.exp (test_forward_search): Rename to ...
+	(test_forward_reverse_search): ... this.  Also test reverse-search
+	and the forward-search alias.
+
 2018-12-05  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* config/sim.exp (gdb_target_sim): Remove redundant adjustment of

gdb/testsuite/gdb.base/list.exp

@@ -485,7 +485,9 @@ proc test_list_filename_and_function {} {
 
 }
 
-proc test_forward_search {} {
+# Test the forward-search (aka search) and the reverse-search commands.
+
+proc test_forward_reverse_search {} {
 	global timeout
 
 	gdb_test_no_output "set listsize 4"
@@ -499,6 +501,17 @@ proc test_forward_search {} {
 
 	gdb_test "search 6789" "28\[ \t\]+oof .6789.;"
 
+	# Try again, we shouldn't re-find the same source line.  Also,
+	# while at it, test using the "forward-search" alias.
+	gdb_test "forward-search 6789" " not found"
+
+	# Now test backwards.  First make sure we start searching from
+	# the previous line, not the current line.
+	gdb_test "reverse-search 6789" " not found"
+
+	# Now find something in a previous line.
+	gdb_test "reverse-search 67" "26\[ \t\]+oof .67.;"
+
 	# Test that GDB won't crash if the line being searched is extremely long.
 
 	set oldtimeout $timeout
@@ -553,7 +566,7 @@ if [ set_listsize 10 ] then {
     test_repeat_list_command
     test_list_range
     test_list_filename_and_function
-    test_forward_search
+    test_forward_reverse_search
     test_only_end
     test_list_invalid_args
 }