From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 49345 invoked by alias); 22 Sep 2017 17:37:21 -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 49336 invoked by uid 89); 22 Sep 2017 17:37:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 22 Sep 2017 17:37:19 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D17EC7F3F0 for ; Fri, 22 Sep 2017 17:37:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D17EC7F3F0 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=sergiodj@redhat.com Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id AC1E05D9CA; Fri, 22 Sep 2017 17:37:17 +0000 (UTC) From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches Subject: Re: [PATCH v3 3/5] Introduce gdb_tilde_expand References: <20170912042325.14927-1-sergiodj@redhat.com> <20170921225926.23132-1-sergiodj@redhat.com> <20170921225926.23132-4-sergiodj@redhat.com> <2045d081-c89b-e83f-97a7-d4e3397aa476@redhat.com> Date: Fri, 22 Sep 2017 17:37:00 -0000 In-Reply-To: <2045d081-c89b-e83f-97a7-d4e3397aa476@redhat.com> (Pedro Alves's message of "Fri, 22 Sep 2017 12:57:30 +0100") Message-ID: <87d16i4otu.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00684.txt.bz2 On Friday, September 22 2017, Pedro Alves wrote: > 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 >> #include >> #include >> @@ -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 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 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... Yeah, you're right. I still intend to keep the cleanups, if that's OK for you. >> +/* 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. Yes, but I think it pays to be explicit in this case. > 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). Or I could simply remove GLOB_ONLYDIR (which is a leftover from the previous versions of the patch) and make this function generic. In fact, I think I prefer this approach, because according to the manpage: GLOB_ONLYDIR This is a hint to glob() that the caller is interested only in directories that match the pattern. If the implementation can easily determine file-type information, then nondirectory files are not returned to the caller. However, the caller must still check that returned files are directories. (The purpose of this flag is merely to optimize performance when the caller is interested only in directories.) I.e., it's only a helper flag for "glob". I'll remove GLOB_ONLYDIR and resubmit the patch. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/