From 73e8dc90a8a94bc52e29596b1cc176b882fbbc8e Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Sat, 8 Dec 2018 15:03:29 +0000 Subject: [PATCH] Merge forward-search/reverse-search, use gdb::def_vector, remove limit Back in: commit 85ae1317add94adef4817927e89cff80b92813dd Author: Stan Shebs 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. 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 * source.c (forward_search_command): Rename to ... (search_command_helper): ... this. Add 'forward' parameter. Tweak to use a gdb::def_vector 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 * 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 | 9 ++ gdb/source.c | 149 ++++++++++---------------------- gdb/testsuite/ChangeLog | 6 ++ gdb/testsuite/gdb.base/list.exp | 17 +++- 4 files changed, 74 insertions(+), 107 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 2e105ae679..df49408b8b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,12 @@ +2018-12-08 Pedro Alves + + * source.c (forward_search_command): Rename to ... + (search_command_helper): ... this. Add 'forward' parameter. + Tweak to use a gdb::def_vector 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 * .dir-locals.el: Copy most of the settings from c-mode over to diff --git a/gdb/source.c b/gdb/source.c index c75351e65f..952fc3f889 100644 --- a/gdb/source.c +++ b/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; - int line; - char *msg; - - line = last_line_listed + 1; - - msg = (char *) re_comp (regex); + const char *msg = 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 buf; + buf.reserve (256); + while (1) { - static char *buf = NULL; - char *p; - int cursize, newsize; - - cursize = 256; - buf = (char *) xrealloc (buf, cursize); - p = buf; + buf.resize (0); - c = fgetc (stream.get ()); + int c = fgetc (stream.get ()); if (c == EOF) break; do { - *p++ = c; - if (p - buf == cursize) - { - newsize = cursize + cursize / 2; - buf = (char *) xrealloc (buf, newsize); - p = buf + cursize; - cursize = newsize; - } + buf.push_back (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') + size_t sz = buf.size (); + if (sz >= 2 && buf[sz - 2] == '\r') { - p--; - p[-1] = '\n'; + buf[sz - 2] = '\n'; + buf.resize (sz - 1); } /* We now have a source line in buf, null terminate and match. */ - *p = 0; - if (re_exec (buf) > 0) + buf.push_back ('\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 -reverse_search_command (const char *regex, int from_tty) +forward_search_command (const char *regex, int from_tty) { - int c; - 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); - } - } + search_command_helper (regex, from_tty, true); +} - printf_filtered (_("Expression not found\n")); - return; +static void +reverse_search_command (const char *regex, int from_tty) +{ + search_command_helper (regex, from_tty, false); } /* If the last character of PATH is a directory separator, then strip it. */ diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 66310d79f4..b6c6c4a1ce 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2018-12-08 Pedro Alves + + * 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 * config/sim.exp (gdb_target_sim): Remove redundant adjustment of diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp index abfb0e165d..9b172e9511 100644 --- a/gdb/testsuite/gdb.base/list.exp +++ b/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 } -- 2.34.1