[RFC] Show (tilde-)expanded filenames to the user? (was: Re: [PATCH] gcore: expand tilde in filename, like in "dump memory" command.)

[Pedro Alves <palves at redhat dot com>]

On 08/07/2013 10:04 PM, Azat Khuzhin wrote:
> Before this patch next command will fail:
> (gdb) generate-core-file ~/core
> Failed to open '~/core' for output.
> 
> After this patch:
> (gdb) generate-core-file ~/core
> Saved corefile ~/core

While reviewing Azat's patch, I wondered whether
GDB should instead display the expanded filename here,
like:

 (gdb) gcore ~/core
 Saved corefile /home/pedro/core

There are many examples of commands that show the
expanded filename, like e.g.,:

 (gdb) file ~/foo
 A program is being debugged already.
 Are you sure you want to change the file? (y or n) y
 Load new symbol table from "/home/pedro/foo"? (y or n) 

or "info files", "info inferiors", and probably a bunch
of others.

I started adjusting his patch, but then noticed we
have cases where we have code that shows the non-expanded
filename to the user, like "save breakpoints", and it
might have been written that way on purpose:

  pathname = tilde_expand (filename);
  cleanup = make_cleanup (xfree, pathname);
  fp = gdb_fopen (pathname, "w");
  if (!fp)
    error (_("Unable to open file '%s' for saving (%s)"),
	   filename, safe_strerror (errno));
...
  if (from_tty)
    printf_filtered (_("Saved to file '%s'.\n"), filename);


(gdb) save breakpoints ~/bp
Saved to file '~/bp'.

So I plan to check his patch in as is first.

But, I think we should have a policy here, and all commands
should follow it.  Those that don't would be considered
bugs.  The question then is, which policy is most appropriate?
grepping around for "tilde_expand", it seems to be the
showing expanded filenames is more common.  Particularly,
whenever we stash the filename in some structure (objfiles,
DSOs, etc.), I don't think ever bother to store the non-expanded
name, so it looks like the only cases we show non-expanded
names are in high level command errors and output, (parse
argument, try to open file).

The patch below applies on top of Azat's and makes GDB show
the expanded filename with "gcore" too, in case that's the
policy to follow.

WDYT?

------
gcore: Make tilde-expanded filename visible.

Before:

  (gdb) generate-core-file ~/core
  Failed to open '~/core' for output.

After:

  (gdb) generate-core-file ~/core
  Saved corefile ~/core

gdb/
2013-08-08  Pedro Alves  <palves@redhat.com>

	* gcore.c (create_gcore_bfd): Don't use tilde_expand here.
	(gcore_command): Use tilde_expand here, and when showing the
	filename to the user, show the expanded version.
---

diff --git a/gdb/gcore.c b/gdb/gcore.c
index 9c83ec8..327aa8e 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -52,12 +52,7 @@ static int gcore_memory_sections (bfd *);
 bfd *
 create_gcore_bfd (const char *filename)
 {
-  char *fullname;
-  bfd *obfd;
-
-  fullname = tilde_expand (filename);
-  obfd = gdb_bfd_openw (fullname, default_gcore_target ());
-  xfree (fullname);
+  bfd *obfd = gdb_bfd_openw (fullname, default_gcore_target ());
 
   if (!obfd)
     error (_("Failed to open '%s' for output."), filename);
@@ -127,8 +122,9 @@ do_bfd_delete_cleanup (void *arg)
 static void
 gcore_command (char *args, int from_tty)
 {
-  struct cleanup *old_chain;
-  char *corefilename, corefilename_buffer[40];
+  struct cleanup *filename_chain;
+  struct cleanup *bfd_chain;
+  char *corefilename;
   bfd *obfd;
 
   /* No use generating a corefile without a target process.  */
@@ -136,15 +132,15 @@ gcore_command (char *args, int from_tty)
     noprocess ();
 
   if (args && *args)
-    corefilename = args;
+    corefilename = tilde_expand (args);
   else
     {
       /* Default corefile name is "core.PID".  */
-      xsnprintf (corefilename_buffer, sizeof (corefilename_buffer),
-		 "core.%d", PIDGET (inferior_ptid));
-      corefilename = corefilename_buffer;
+      corefilename = xstrprintf ("core.%d", PIDGET (inferior_ptid));
     }
 
+  filename_chain = make_cleanup (xfree, corefilename);
+
   if (info_verbose)
     fprintf_filtered (gdb_stdout,
 		      "Opening corefile '%s' for output.\n", corefilename);
@@ -153,16 +149,17 @@ gcore_command (char *args, int from_tty)
   obfd = create_gcore_bfd (corefilename);
 
   /* Need a cleanup that will close and delete the file.  */
-  old_chain = make_cleanup (do_bfd_delete_cleanup, obfd);
+  bfd_chain = make_cleanup (do_bfd_delete_cleanup, obfd);
 
   /* Call worker function.  */
   write_gcore_file (obfd);
 
   /* Succeeded.  */
-  fprintf_filtered (gdb_stdout, "Saved corefile %s\n", corefilename);
-
-  discard_cleanups (old_chain);
+  discard_cleanups (bfd_chain);
   gdb_bfd_unref (obfd);
+
+  fprintf_filtered (gdb_stdout, "Saved corefile %s\n", corefilename);
+  do_cleanups (filename_chain);
 }
 
 static unsigned long