From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43351 invoked by alias); 13 Sep 2017 22:03:40 -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 43326 invoked by uid 89); 13 Sep 2017 22:03:39 -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 22:03:37 +0000 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BAB697EBD5 for ; Wed, 13 Sep 2017 22:03:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BAB697EBD5 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.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 935E360BE4; Wed, 13 Sep 2017 22:03:36 +0000 (UTC) From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches Subject: Re: [PATCH 1/4] Make gdb_dirbuf local to functions References: <20170912042325.14927-1-sergiodj@redhat.com> <20170912042325.14927-2-sergiodj@redhat.com> <0e75c4bf-c9ec-7a07-f1e0-c9d7f11dd136@redhat.com> Date: Wed, 13 Sep 2017 22:03:00 -0000 In-Reply-To: <0e75c4bf-c9ec-7a07-f1e0-c9d7f11dd136@redhat.com> (Pedro Alves's message of "Wed, 13 Sep 2017 16:12:11 +0100") Message-ID: <87a81yz26g.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/msg00365.txt.bz2 On Wednesday, September 13 2017, Pedro Alves wrote: > 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? BUFSIZ is defined by stdio.h, and is 8192 on my Fedora 25. This is much greater than the previous "1024" that was being used before. For reproducibility reasons I didn't want to use PATH_MAX. >> + >> 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. That's true, and it's better when you consider reproducible builds as well. According to gnulib: https://www.gnu.org/software/gnulib/manual/html_node/getcwd.html It is better to rely on this better version of getcwd. 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/