Re: [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)

[iam ahal <hal9000ed2k@gmail.com>]

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

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.

On Wed, Aug 3, 2011 at 9:44 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Eldar" == iam ahal <hal9000ed2k@gmail.com> writes:
>
> Tom> I forget (I never keep records of this, I think perhaps I should) -- did
> Tom> we get you started on the copyright assignment paperwork?
>
> Eldar> Not yet, because my previous patches is very different. I mean the
> Eldar> different files was changed and I don't know which files I need to
> Eldar> change in the next patch.
> Eldar> I hope some people here can accept implementation in this last patch.
> Eldar> After I can start the copyright assignment.
>
> It is best to start early.  Sometimes it can take quite a while, and in
> the meantime your patch will not go in.
>
> The list of files does not really matter, since the assignment is for
> past and future changes.  You can ask the copyright clerk for
> clarification on this point if you need it.
>
> I think the patch is basically fine, just a few nits.
>
> Eldar> +static int backtrace_skip_compile;
> Eldar> +static void
>
> Tom> Newline between these two lines.
>
> Eldar> I'm not sure that's right because I see that there's no newline in
> Eldar> this case (if you look at other places related 'set backtrace ...').
>
> Yeah, I think a newline is better, though.
>
> Eldar> +char *
> Eldar> +get_display_filename_from_sal (struct symtab_and_line *sal)
> Tom>
> Tom> Should have an introductory comment before this function.
>
> Eldar> Comment was added in 'frame.h'
>
> Ok, thanks.
> Add a comment saying to see the documentation in frame.h.
>
> Eldar> +static const char filename_display_without_comp_dir[] = "without-compile-dir";
>
> I think spelling out "directory" would be better.
>
> Eldar> +  const char *filename = sal->symtab->filename;
> Eldar> +  const char *dirname = sal->symtab->dirname;
> Eldar> +  size_t flen = strlen (filename);
> Eldar> +  size_t dlen = strlen (dirname);
>
> This will crash if filename==NULL or dirname==NULL.
>
> Eldar> +  if (filename_display_string == filename_display_without_comp_dir
> Eldar> +      && filename && dirname && dlen <= flen
> Eldar> +      && !FILENAME_NCMP (filename, dirname, dlen))
>
> This test is not correct, in that it will match "/tmp/x" with "/tmp/xaaa".
> I think it needs an additional check for a separator.
>
> Eldar> diff -rup gdb-7.2-orig/include/filenames.h gdb-7.2/include/filenames.h
>
> Changes to libiberty have to go to gcc-patches.
> I don't think you actually need this change, though, so it would be
> simpler and quicker if you left it out.
>
> Eldar> +extern int filename_ncmp (const char *s1, const char *s2, size_t n);
>
> This is already declared in the trunk filenames.h.
> Be sure to always write patches against the trunk, not something older.
>
> Tom
>

[-- Attachment #2: gdb-7.3.1-filename-display-v1.patch --]
[-- Type: text/x-patch, Size: 6012 bytes --]

diff -rup gdb-7.3.1-orig/gdb/doc/gdb.texinfo gdb-7.3.1/gdb/doc/gdb.texinfo
--- gdb-7.3.1-orig/gdb/doc/gdb.texinfo	2011-09-04 21:10:37.000000000 +0400
+++ gdb-7.3.1/gdb/doc/gdb.texinfo	2011-10-30 21:26:34.450274962 +0400
@@ -6052,6 +6052,19 @@ unlimited.
 
 @item show backtrace limit
 Display the current limit on backtrace levels.
+
+@item set backtrace filename-display
+@itemx set backtrace filename-display full
+Display a full filename.  This is the default.
+
+@item set backtrace filename-display basename
+Display only basename of a filename.
+
+@item set backtrace filename-display basename
+Display a filename without the compile directory part.
+
+@item show backtrace filename-display
+Display the current way to display a filename.
 @end table
 
 @node Selection
diff -rup gdb-7.3.1-orig/gdb/frame.c gdb-7.3.1/gdb/frame.c
--- gdb-7.3.1-orig/gdb/frame.c	2011-03-18 21:52:30.000000000 +0300
+++ gdb-7.3.1/gdb/frame.c	2011-10-30 21:28:45.170271140 +0400
@@ -45,6 +45,7 @@
 #include "block.h"
 #include "inline-frame.h"
 #include  "tracepoint.h"
+#include "filenames.h"
 
 static struct frame_info *get_prev_frame_1 (struct frame_info *this_frame);
 static struct frame_info *get_prev_frame_raw (struct frame_info *this_frame);
@@ -136,6 +137,18 @@ struct frame_info
    sufficient for now.  */
 static struct frame_info *frame_stash = NULL;
 
+/* Possible values of 'set backtrace filename-display'.  */
+static const char filename_display_full[] = "full";
+static const char filename_display_basename[] = "basename";
+static const char filename_display_without_comp_directory[] = "without-compile-directory";
+
+static const char *filename_display_kind_names[] = {
+  filename_display_full,
+  filename_display_basename,
+  filename_display_without_comp_directory,
+  NULL
+};
+
 /* Add the following FRAME to the frame stash.  */
 
 static void
@@ -208,6 +221,16 @@ show_backtrace_limit (struct ui_file *fi
 		    value);
 }
 
+static const char *filename_display_string = filename_display_full;
+
+static void
+show_filename_display_string (struct ui_file *file, int from_tty,
+			      struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file,
+		    _("A filename is displayed in backtrace as \"%s\".\n"),
+		    value);
+}
 
 static void
 fprint_field (struct ui_file *file, const char *name, int p, CORE_ADDR addr)
@@ -2113,6 +2136,39 @@ find_frame_sal (struct frame_info *frame
   (*sal) = find_pc_line (pc, notcurrent);
 }
 
+/* See commentary in frame.h.  */
+
+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);
+    }
+  else
+  if (filename_display_string == filename_display_without_comp_directory
+      && filename && dirname && dlen && dlen <= strlen (filename)
+      && !filename_ncmp (filename, dirname, dlen))
+    {
+      const char *start = filename + dlen;
+      const char *result = start;
+      while (IS_DIR_SEPARATOR (*result))
+	result++;
+
+      if (IS_DIR_SEPARATOR (dirname[dlen - 1]))
+	return result;
+      else
+	return result == start ? filename : result;
+    }
+
+  return filename;
+}
+
 /* Per "frame.h", return the ``address'' of the frame.  Code should
    really be using get_frame_id().  */
 CORE_ADDR
@@ -2484,6 +2540,21 @@ Zero is unlimited."),
 			   &set_backtrace_cmdlist,
 			   &show_backtrace_cmdlist);
 
+  add_setshow_enum_cmd ("filename-display", class_obscure,
+			filename_display_kind_names,
+			&filename_display_string, _("\
+Set a way how to display filename."), _("\
+Show a way how to display filename."), _("\
+filename-display can be:\n\
+  full                       - display a full filename, this is the default\n\
+  basename                   - display only basename of a filename\n\
+  without-compile-directory  - display a filename without the compile directory part\n\
+By default, full filename is displayed."),
+			NULL,
+			show_filename_display_string,
+			&set_backtrace_cmdlist,
+			&show_backtrace_cmdlist);
+
   /* Debug this files internals.  */
   add_setshow_zinteger_cmd ("frame", class_maintenance, &frame_debug,  _("\
 Set frame debugging."), _("\
diff -rup gdb-7.3.1-orig/gdb/frame.h gdb-7.3.1/gdb/frame.h
--- gdb-7.3.1-orig/gdb/frame.h	2011-03-18 21:52:30.000000000 +0300
+++ gdb-7.3.1/gdb/frame.h	2011-10-30 20:53:23.178333185 +0400
@@ -352,6 +352,12 @@ extern int get_frame_func_if_available (
 extern void find_frame_sal (struct frame_info *frame,
 			    struct symtab_and_line *sal);
 
+/* Returns either full filename or basename or filename
+   without compile directory part.
+   It depends on 'set backtrace filename-display' value.  */
+
+extern const char *get_filename_display_from_sal (struct symtab_and_line *sal);
+
 /* Set the current source and line to the location given by frame
    FRAME, if possible.  When CENTER is true, adjust so the relevant
    line is in the center of the next 'list'.  */
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);

[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 839 bytes --]

2011-10-30  Eldar Gaynetdinov <hal9000ed2k@gmail.com>

	Add a new variable that controls a way in which filenames in
	backtraces is displayed.
	* gdb.texinfo: Added description of 'filename-display' variable in
	'set/show backtrace' section.
	* 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.
	(show_filename_display_string): New function.
	(get_filename_display_from_sal): New function with commentary.
	(_initialize_frame): Added initialization of 'filename-display'
	variable.
	* frame.h: Added declaration of a new function with commentary.
	* 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.