This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 3/5] Introduce gdb_tilde_expand
- From: Pedro Alves <palves at redhat dot com>
- To: Sergio Durigan Junior <sergiodj at redhat dot com>, GDB Patches <gdb-patches at sourceware dot org>
- Date: Fri, 22 Sep 2017 12:57:30 +0100
- Subject: Re: [PATCH v3 3/5] Introduce gdb_tilde_expand
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9F44C267C8
- References: <20170912042325.14927-1-sergiodj@redhat.com> <20170921225926.23132-1-sergiodj@redhat.com> <20170921225926.23132-4-sergiodj@redhat.com>
On 09/21/2017 11:59 PM, Sergio Durigan Junior wrote:
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index cbafb13837..507fdc4120 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -54,6 +54,8 @@
> #include "tui/tui.h" /* For tui_active et.al. */
> #endif
>
> +#include "gdb_tilde_expand.h"
> +
> #include <fcntl.h>
> #include <algorithm>
> #include <string>
> @@ -410,10 +412,8 @@ cd_command (char *dir, int from_tty)
> repeat might be useful but is more likely to be a mistake. */
> dont_repeat ();
>
> - gdb::unique_xmalloc_ptr<char> dir_holder
> - (tilde_expand (dir != NULL ? dir : "~"));
> - dir = dir_holder.get ();
> -
> + std::string expanded_path = gdb_tilde_expand (dir != NULL ? dir : "~");
> + dir = (char *) expanded_path.c_str ();
> if (chdir (dir) < 0)
> perror_with_name (dir);
>
> @@ -438,7 +438,7 @@ cd_command (char *dir, int from_tty)
> len--;
> }
>
> - dir_holder.reset (savestring (dir, len));
> + gdb::unique_xmalloc_ptr<char> dir_holder (savestring (dir, len));
> if (IS_ABSOLUTE_PATH (dir_holder.get ()))
> {
> xfree (current_directory);
I realized something: in light of the fact that "cd" is not what
is used to specify the inferior's cwd anymore since v1, patching
this particular use of tilde_expand, and not others seems arbitrary.
I.e., this now looks like kind of a spurious change to me, and
I think you should drop the changes to this file...
> +/* See common/gdb_tilde_expand.h. */
> +
> +std::string
> +gdb_tilde_expand (const char *dir)
> +{
> + gdb_glob glob (dir, GLOB_TILDE | GLOB_TILDE_CHECK | GLOB_ONLYDIR, NULL);
By my reading of man glob, GLOB_TILDE_CHECK already implies GLOB_TILDE.
I'm not sure whether GLOB_ONLYDIR is the right thing to do in
a function whose name doesn't suggest that it concern itself
with anything other than tilde expansion. E.g., if we replaced
tilde_expand with gdb_tilde_expand in place that expect to match
files (like e.g., gdb_print_filename), then would this work as is?
Maybe you should rename this to gdb_tilde_expand_dir (leaving
the file names as is).
Thanks,
Pedro Alves