This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 2/9] Code cleanup: Drop IS_ABSOLUTE_PATH checks
On Sat, 19 Jan 2013 16:27:31 +0100, Jan Kratochvil wrote:
> I will try to add some comment there in a next patch.
Done.
Thanks,
Jan
gdb/
2013-01-15 Jan Kratochvil <jan.kratochvil@redhat.com>
Code cleanup.
* breakpoint.c (clear_command): Remove variable is_abs, unify the
call of filename_cmp with compare_filenames_for_search.
* dwarf2read.c (dw2_map_symtabs_matching_filename): Remove variable
is_abs, unify the call of FILENAME_CMP with
compare_filenames_for_search. New gdb_asserts for full_path and
real_path. Unify the call of compare_filenames_for_search with
FILENAME_CMP.
* psymtab.c (partial_map_symtabs_matching_filename): Likewise.
* symfile.h (struct quick_symbol_functions): Extend the comment for
map_symtabs_matching_filename.
* symtab.c (compare_filenames_for_search): Remove the function comment
relative path requirement. Handle absolute filenames.
(iterate_over_some_symtabs): Remove variable is_abs, unify the call of
FILENAME_CMP with compare_filenames_for_search. New gdb_asserts for
full_path and real_path. Unify the call of
compare_filenames_for_search with FILENAME_CMP.
gdb/testsuite/
2013-01-18 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.mi/mi-fullname-deleted.exp: Use double last slash for $srcfileabs.
(compare_filenames_for_search does not match)
(compare_filenames_for_search does match): New tests.
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -11909,8 +11909,6 @@ clear_command (char *arg, int from_tty)
make_cleanup (VEC_cleanup (breakpoint_p), &found);
for (i = 0; i < sals.nelts; i++)
{
- int is_abs;
-
/* If exact pc given, clear bpts at that pc.
If line given (pc == 0), clear all bpts on specified line.
If defaulting, clear all bpts on default line
@@ -11924,7 +11922,6 @@ clear_command (char *arg, int from_tty)
1 0 <can't happen> */
sal = sals.sals[i];
- is_abs = sal.symtab == NULL ? 1 : IS_ABSOLUTE_PATH (sal.symtab->filename);
/* Find all matching breakpoints and add them to 'found'. */
ALL_BREAKPOINTS (b)
@@ -11952,12 +11949,8 @@ clear_command (char *arg, int from_tty)
&& sal.pspace == loc->pspace
&& loc->line_number == sal.line)
{
- if (filename_cmp (loc->symtab->filename,
- sal.symtab->filename) == 0)
- line_match = 1;
- else if (!IS_ABSOLUTE_PATH (sal.symtab->filename)
- && compare_filenames_for_search (loc->symtab->filename,
- sal.symtab->filename))
+ if (compare_filenames_for_search (loc->symtab->filename,
+ sal.symtab->filename))
line_match = 1;
}
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3034,7 +3034,6 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
{
int i;
const char *name_basename = lbasename (name);
- int is_abs = IS_ABSOLUTE_PATH (name);
dw2_setup (objfile);
@@ -3059,8 +3058,7 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
{
const char *this_name = file_data->file_names[j];
- if (FILENAME_CMP (name, this_name) == 0
- || (!is_abs && compare_filenames_for_search (this_name, name)))
+ if (compare_filenames_for_search (this_name, name))
{
if (dw2_map_expand_apply (objfile, per_cu,
name, full_path, real_path,
@@ -3079,11 +3077,10 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
const char *this_real_name = dw2_get_real_path (objfile,
file_data, j);
+ gdb_assert (IS_ABSOLUTE_PATH (full_path));
+ gdb_assert (IS_ABSOLUTE_PATH (name));
if (this_real_name != NULL
- && (FILENAME_CMP (full_path, this_real_name) == 0
- || (!is_abs
- && compare_filenames_for_search (this_real_name,
- name))))
+ && FILENAME_CMP (full_path, this_real_name) == 0)
{
if (dw2_map_expand_apply (objfile, per_cu,
name, full_path, real_path,
@@ -3097,11 +3094,10 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
const char *this_real_name = dw2_get_real_path (objfile,
file_data, j);
+ gdb_assert (IS_ABSOLUTE_PATH (real_path));
+ gdb_assert (IS_ABSOLUTE_PATH (name));
if (this_real_name != NULL
- && (FILENAME_CMP (real_path, this_real_name) == 0
- || (!is_abs
- && compare_filenames_for_search (this_real_name,
- name))))
+ && FILENAME_CMP (real_path, this_real_name) == 0)
{
if (dw2_map_expand_apply (objfile, per_cu,
name, full_path, real_path,
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -168,7 +168,6 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
{
struct partial_symtab *pst;
const char *name_basename = lbasename (name);
- int is_abs = IS_ABSOLUTE_PATH (name);
ALL_OBJFILE_PSYMTABS_REQUIRED (objfile, pst)
{
@@ -181,8 +180,7 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
if (pst->anonymous)
continue;
- if (FILENAME_CMP (name, pst->filename) == 0
- || (!is_abs && compare_filenames_for_search (pst->filename, name)))
+ if (compare_filenames_for_search (pst->filename, name))
{
if (partial_map_expand_apply (objfile, name, full_path, real_path,
pst, callback, data))
@@ -199,11 +197,11 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
this symtab and use its absolute path. */
if (full_path != NULL)
{
+ gdb_assert (IS_ABSOLUTE_PATH (full_path));
+ gdb_assert (IS_ABSOLUTE_PATH (name));
psymtab_to_fullname (pst);
if (pst->fullname != NULL
- && (FILENAME_CMP (full_path, pst->fullname) == 0
- || (!is_abs && compare_filenames_for_search (pst->fullname,
- name))))
+ && FILENAME_CMP (full_path, pst->fullname) == 0)
{
if (partial_map_expand_apply (objfile, name, full_path, real_path,
pst, callback, data))
@@ -214,15 +212,16 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
if (real_path != NULL)
{
char *rp = NULL;
+
+ gdb_assert (IS_ABSOLUTE_PATH (real_path));
+ gdb_assert (IS_ABSOLUTE_PATH (name));
psymtab_to_fullname (pst);
if (pst->fullname != NULL)
{
rp = gdb_realpath (pst->fullname);
make_cleanup (xfree, rp);
}
- if (rp != NULL
- && (FILENAME_CMP (real_path, rp) == 0
- || (!is_abs && compare_filenames_for_search (real_path, name))))
+ if (rp != NULL && FILENAME_CMP (real_path, rp) == 0)
{
if (partial_map_expand_apply (objfile, name, full_path, real_path,
pst, callback, data))
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -159,9 +159,11 @@ struct quick_symbol_functions
/* Expand and iterate over each "partial" symbol table in OBJFILE
where the source file is named NAME.
- If NAME is not absolute, a match after a '/' in the symbol
- table's file name will also work. FULL_PATH is the absolute file
- name, and REAL_PATH is the same, run through gdb_realpath.
+ If NAME is not absolute, a match after a '/' in the symbol table's
+ file name will also work, both FULL_PATH and REAL_PATH are NULL
+ then. If NAME is absolute then both FULL_PATH and REAL_PATH are
+ non-NULL absolute file names. FULL_PATH is NAME resolved via
+ xfullpath, REAL_PATH is NAME resolved via gdb_realpath.
If a match is found, the "partial" symbol table is expanded.
Then, this calls iterate_over_some_symtabs (or equivalent) over
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -146,8 +146,8 @@ const struct block *block_found;
/* See whether FILENAME matches SEARCH_NAME using the rule that we
advertise to the user. (The manual's description of linespecs
- describes what we advertise). We assume that SEARCH_NAME is
- a relative path. Returns true if they match, false otherwise. */
+ describes what we advertise). Returns true if they match, false
+ otherwise. */
int
compare_filenames_for_search (const char *filename, const char *search_name)
@@ -166,12 +166,18 @@ compare_filenames_for_search (const char *filename, const char *search_name)
preceding the trailing SEARCH_NAME segment of FILENAME must be a
directory separator.
+ The check !IS_ABSOLUTE_PATH ensures SEARCH_NAME "/dir/file.c"
+ cannot match FILENAME "/path//dir/file.c" - as user has requested
+ absolute path. The sama applies for "c:\file.c" possibly
+ incorrectly hypothetically matching "d:\dir\c:\file.c".
+
The HAS_DRIVE_SPEC purpose is to make FILENAME "c:file.c"
compatible with SEARCH_NAME "file.c". In such case a compiler had
to put the "c:file.c" name into debug info. Such compatibility
works only on GDB built for DOS host. */
return (len == search_len
- || IS_DIR_SEPARATOR (filename[len - search_len - 1])
+ || (!IS_ABSOLUTE_PATH (search_name)
+ && IS_DIR_SEPARATOR (filename[len - search_len - 1]))
|| (HAS_DRIVE_SPEC (filename)
&& STRIP_DRIVE_SPEC (filename) == &filename[len - search_len]));
}
@@ -199,18 +205,10 @@ iterate_over_some_symtabs (const char *name,
{
struct symtab *s = NULL;
const char* base_name = lbasename (name);
- int is_abs = IS_ABSOLUTE_PATH (name);
for (s = first; s != NULL && s != after_last; s = s->next)
{
- /* Exact match is always ok. */
- if (FILENAME_CMP (name, s->filename) == 0)
- {
- if (callback (s, data))
- return 1;
- }
-
- if (!is_abs && compare_filenames_for_search (s->filename, name))
+ if (compare_filenames_for_search (s->filename, name))
{
if (callback (s, data))
return 1;
@@ -229,17 +227,13 @@ iterate_over_some_symtabs (const char *name,
{
const char *fp = symtab_to_fullname (s);
+ gdb_assert (IS_ABSOLUTE_PATH (full_path));
+ gdb_assert (IS_ABSOLUTE_PATH (name));
if (FILENAME_CMP (full_path, fp) == 0)
{
if (callback (s, data))
return 1;
}
-
- if (!is_abs && compare_filenames_for_search (fp, name))
- {
- if (callback (s, data))
- return 1;
- }
}
if (real_path != NULL)
@@ -248,6 +242,8 @@ iterate_over_some_symtabs (const char *name,
char *rp = gdb_realpath (fullname);
struct cleanup *cleanups = make_cleanup (xfree, rp);
+ gdb_assert (IS_ABSOLUTE_PATH (real_path));
+ gdb_assert (IS_ABSOLUTE_PATH (name));
if (FILENAME_CMP (real_path, rp) == 0)
{
if (callback (s, data))
@@ -256,15 +252,6 @@ iterate_over_some_symtabs (const char *name,
return 1;
}
}
-
- if (!is_abs && compare_filenames_for_search (rp, name))
- {
- if (callback (s, data))
- {
- do_cleanups (cleanups);
- return 1;
- }
- }
do_cleanups (cleanups);
}
}
--- a/gdb/testsuite/gdb.mi/mi-fullname-deleted.exp
+++ b/gdb/testsuite/gdb.mi/mi-fullname-deleted.exp
@@ -24,6 +24,12 @@ if [mi_gdb_start] {
standard_testfile
set srcfileabs [standard_output_file $srcfile]
+# "//$srcfile" It is used for the test of compare_filenames_for_search.
+if { [regsub {/[^/]+$} $srcfileabs {/\0} srcfileabs] != 1 } {
+ xfail "Cannot double the last slash separator"
+ return -1
+}
+
if { [regsub {^(/[^/]+)/} $srcfileabs {\1subst/} srcfileabssubst] != 1
|| [regsub {^(/[^/]+)/.*$} $srcfileabs {\1} initdir] != 1 } {
xfail "Missing root subdirectory"
@@ -49,3 +55,12 @@ mi_gdb_test "-interpreter-exec console \"set substitute-path ${initdir} ${initdi
mi_gdb_test "-file-list-exec-source-file" ".*\",fullname=\".*\".*" "fullname present"
mi_gdb_test "-file-list-exec-source-file" ".*\",fullname=\"[string_to_regexp $srcfileabssubst]\".*" "substituted fullname"
+
+# Test compare_filenames_for_search does not falsely use absolute filename as
+# a relative one.
+mi_gdb_test "-break-insert -t /$srcfile:main" \
+ "\\^error,msg=\"No source file named /[string_to_regexp $srcfile]\\.\"" \
+ "compare_filenames_for_search does not match"
+mi_gdb_test "-break-insert -t $srcfile:main" \
+ {\^done,bkpt=.*} \
+ "compare_filenames_for_search does match"