From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89419 invoked by alias); 13 Sep 2017 15:12:25 -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 88873 invoked by uid 89); 13 Sep 2017 15:12:24 -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; Wed, 13 Sep 2017 15:12:23 +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 88D17C0587F7 for ; Wed, 13 Sep 2017 15:12:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 88D17C0587F7 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves@redhat.com Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id C796B60C34; Wed, 13 Sep 2017 15:12:14 +0000 (UTC) Subject: Re: [PATCH 1/4] Make gdb_dirbuf local to functions To: Sergio Durigan Junior , GDB Patches References: <20170912042325.14927-1-sergiodj@redhat.com> <20170912042325.14927-2-sergiodj@redhat.com> From: Pedro Alves Message-ID: <0e75c4bf-c9ec-7a07-f1e0-c9d7f11dd136@redhat.com> Date: Wed, 13 Sep 2017 15:12:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20170912042325.14927-2-sergiodj@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-09/txt/msg00361.txt.bz2 On 09/12/2017 05:23 AM, Sergio Durigan Junior wrote: > This is not a requirement for the feature, but I think it's a good > cleanup and is related anyway. Currently we have "current_directory" > and "gdb_dirbuf" globals, which means that we basically have two > possible places to consult when we want to know GDB's current working > directory. > > This is not ideal and can lead to confusion, so my proposal is to > "internalize" the "gdb_dirbuf" variable, because it is really just > that: a buffer to be used when calling "getcwd". With this, only > "current_directory" should be used to determine the cwd. > > gdb/ChangeLog: > yyyy-mm-dd Sergio Durigan Junior > > * cli/cli-cmds.c (quit_command): Declare "gdb_dirbuf". > (cd_command): Free "current_directory" before assigning to it. > * main.c (captured_main_1): Likewise. Call "xstrdup" when > storing it on "current_directory". > * mi/mi-cmd-env.c (mi_cmd_env_pwd): Declare "gdb_dirbuf". > * top.c (gdb_dirbuf): Remove global declaration. > * top.h (gdb_dirbuf): Likewise. > --- > gdb/cli/cli-cmds.c | 7 ++++++- > gdb/main.c | 4 +++- > gdb/mi/mi-cmd-env.c | 1 + > gdb/top.c | 3 --- > gdb/top.h | 1 - > 5 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index 883844ee70..64e893c784 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -380,6 +380,8 @@ quit_command (char *args, int from_tty) > static void > pwd_command (char *args, int from_tty) > { > + char gdb_dirbuf[BUFSIZ]; I don't recall offhand -- what's "BUFSIZ"? What defines it and is it guaranteed to be the right size for getcwd? > + > if (args) > error (_("The \"pwd\" command does not take an argument: %s"), args); > if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf))) > @@ -434,7 +436,10 @@ cd_command (char *dir, int from_tty) > > dir_holder.reset (savestring (dir, len)); > if (IS_ABSOLUTE_PATH (dir_holder.get ())) > - current_directory = dir_holder.release (); > + { > + xfree (current_directory); > + current_directory = dir_holder.release (); > + } > else > { > if (IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])) > diff --git a/gdb/main.c b/gdb/main.c > index a0646edad6..9837729966 100644 > --- a/gdb/main.c > +++ b/gdb/main.c > @@ -549,10 +549,12 @@ captured_main_1 (struct captured_main_args *context) > (xstrprintf ("%s: warning: ", gdb_program_name)); > warning_pre_print = tmp_warn_preprint.get (); > > + char gdb_dirbuf[BUFSIZ]; > + > if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf))) > perror_warning_with_name (_("error finding working directory")); > > - current_directory = gdb_dirbuf; > + current_directory = xstrdup (gdb_dirbuf); As an extension, on GNU/Linux, you can call 'getcwd (NULL, 0)' and that returns a heap-allocated buffer of the necessary size. Can we rely on gnulib to implement that behavior everywhere? Since it seems like we're xstrdup'ing the dir anyway, that likely would simplify things, and remove one arbitrary hardcoded limit, while at it. Thanks, Pedro Alves