From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17342 invoked by alias); 8 Aug 2013 18:16:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 17316 invoked by uid 89); 8 Aug 2013 18:16:56 -0000 X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00,FREEMAIL_FROM,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RDNS_NONE,SPF_PASS autolearn=ham version=3.3.1 Received: from Unknown (HELO mail-vb0-f41.google.com) (209.85.212.41) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 08 Aug 2013 18:16:54 +0000 Received: by mail-vb0-f41.google.com with SMTP id g17so3446509vbg.14 for ; Thu, 08 Aug 2013 11:16:47 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.221.55.4 with SMTP id vw4mr763448vcb.37.1375985807364; Thu, 08 Aug 2013 11:16:47 -0700 (PDT) Received: by 10.59.6.161 with HTTP; Thu, 8 Aug 2013 11:16:47 -0700 (PDT) In-Reply-To: <5203D88C.8060801@redhat.com> References: <1375909475-16720-1-git-send-email-a3at.mail@gmail.com> <5203D88C.8060801@redhat.com> Date: Thu, 08 Aug 2013 18:16:00 -0000 Message-ID: Subject: Re: [RFC] Show (tilde-)expanded filenames to the user? (was: Re: [PATCH] gcore: expand tilde in filename, like in "dump memory" command.) From: Azat Khuzhin To: Pedro Alves Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2013-08/txt/msg00252.txt.bz2 On Thu, Aug 8, 2013 at 9:42 PM, Pedro Alves wrote: > 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). I agree, the file name must printed in one manner. > > 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 > > * 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 ()); Seems that here must be filename (not fullname) ? > > 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 > -- Respectfully Azat Khuzhin