This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)
- From: Doug Evans <dje at google dot com>
- To: iam ahal <hal9000ed2k at gmail dot com>
- Cc: Tom Tromey <tromey at redhat dot com>, gdb-patches at sourceware dot org, eliz at gnu dot org, pmuldoon at redhat dot com, brobecker at adacore dot com, pedro at codesourcery dot com, drow at false dot org, jan dot kratochvil at redhat dot com
- Date: Wed, 2 Nov 2011 15:53:24 -0700
- Subject: Re: [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)
- References: <BANLkTinD+9_Mkug8o2VhZ03L6XSriL_RKQ@mail.gmail.com> <m3oc1kfheh.fsf@redhat.com> <20110627160029.GF20676@adacore.com> <m3sjqt67pe.fsf@fleche.redhat.com> <m3mxh1oa8a.fsf@redhat.com> <CAA18ubJ10sh3pPDLp4V44qY6r6hLU9RqyDi62KuAAtfXJY7Oug@mail.gmail.com> <834o33qlm9.fsf@gnu.org> <CAA18ubJZK7w51Bmwvy7xYXPvHf7e=bbRbdBOqNvZA-PrXJpUsA@mail.gmail.com> <E1Qdhn8-0000fE-TK@fencepost.gnu.org> <CAA18ubJAvwHt-sq8XN98PhC=LSFXPbcy4P1+AVic-G=MNE7R2A@mail.gmail.com> <CAA18ubLBZseSxqjWSk1jH7OeZi06K_o75HKc0CN-_iCAjQ5boA@mail.gmail.com> <83bowq6x7f.fsf@gnu.org> <CAA18ub+ox5kmHu=1qvMkwNfzbCxMdy3M4Z8eKuheaiqyjxJvEg@mail.gmail.com> <m3d3gu77uk.fsf@fleche.redhat.com> <CAA18ubL6ofB9X+2nJC3jUCTEqZBbWL4=wXHgBkOFKJaAOXsiow@mail.gmail.com> <m3vcue1jal.fsf@fleche.redhat.com> <CAA18ubL9TG7we7bi3U_0DQw8KLXNaYCfGa91KQrvC2jM=6DsBw@mail.gmail.com>
On Sun, Oct 30, 2011 at 11:16 AM, iam ahal <hal9000ed2k@gmail.com> wrote:
> Copyright assignment by [gnu.org #703515] is completed.
> I've corrected some pieces of the patch by your notes.
> Also, I've attached the change log.
>
> Please, check it.
>
> Thank you.
>
> ~Eldar.
Hi.
Here's my $0.02 review.
These are all nits.
---
For your changelog entry:
[these are all for consistency with existing entries]
Add a new variable that controls a way in which filenames in
backtraces is displayed.
For this entry:
* gdb.texinfo: Added description of 'filename-display' variable in
'set/show backtrace' section.
Specify which @node the gdb.texinfo entry is for.
Existing example:
? ? ? ?* gdb.texinfo (Values From Inferior): Add is_lazy attribute,
? ? ? ?fetch_lazy method.
For this entry:
* frame.c: Added including of a header file. ?Added definition of
three global string variables and global array of string variables.
Added definition of one global string variable.
Specify what the variable names are.
Existing example:
? ? ? ?* infrun.c (disable_randomization): New global variable.
(show_filename_display_string): New function.
For this entry:
(get_filename_display_from_sal): New function with commentary.
No need to write "with commentary" here or below.
(_initialize_frame): Added initialization of 'filename-display'
variable.
For this entry:
* frame.h: Added declaration of a new function with commentary.
You need to write the name of the function here.
Existing example:
? ? ? ?* value.h (read_frame_register_value): Add declaration.
* stack.c (print_frame): Added new variable and calling of a new
function and condition with this variable. Changed third argument of
calling of a function.
---
For your patch:
I wish I could think of a shorter name than
"without-compile-directory", but I can't.
OTOH gdb uses "compilation directory" everywhere"
$ grep "compile.*directory" *.[ch] | wc
0 ...
$ grep "compilation.*directory" *.[ch] | wc
9 ...
Can we change this to without-compilation-directory?
[It's already long, and it still tab-completes in one character. :-)]
In get_filename_display_from_sal:
+const char *
+get_filename_display_from_sal (struct symtab_and_line *sal)
+{
+ const char *filename = sal->symtab->filename;
+ const char *dirname = sal->symtab->dirname;
+ size_t dlen = dirname ? strlen (dirname) : 0;
+
+ if (filename_display_string == filename_display_basename
+ && filename)
+ {
+ return lbasename (filename);
+ }
[...]
There are a few "&& filename" checks.
The code would be easier to read if you did
if (filename == NULL)
return NULL;
at the start.
+ else
+ if (filename_display_string == filename_display_without_comp_directory
"else" and "if" go on the same line.
In stack.c:
diff -rup gdb-7.3.1-orig/gdb/stack.c gdb-7.3.1/gdb/stack.c
--- gdb-7.3.1-orig/gdb/stack.c 2011-03-18 21:48:56.000000000 +0300
+++ gdb-7.3.1/gdb/stack.c 2011-10-30 20:53:23.182333185 +0400
@@ -835,11 +835,15 @@ print_frame (struct frame_info *frame, i
ui_out_text (uiout, ")");
if (sal.symtab && sal.symtab->filename)
{
+ const char *filename_display = get_filename_display_from_sal (&sal);
+ if (filename_display == NULL)
+ filename_display = sal.symtab->filename;
+
annotate_frame_source_begin ();
ui_out_wrap_hint (uiout, " ");
ui_out_text (uiout, " at ");
annotate_frame_source_file ();
- ui_out_field_string (uiout, "file", sal.symtab->filename);
+ ui_out_field_string (uiout, "file", filename_display);
if (ui_out_is_mi_like_p (uiout))
{
const char *fullname = symtab_to_fullname (sal.symtab);
If filename_display is NULL it's because sal.symtab->filename was NULL.
[right?]
This is confusing.
I suggest removing this code:
+ if (filename_display == NULL)
+ filename_display = sal.symtab->filename;