[v2,2/5] Get rid of "gdb_dirbuf" and use "getcwd (NULL, 0)"
Commit Message
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. Moreover, the way we're
using "gdb_difbuf" along with "getcwd" is problematic because we
declare the buffer with "1024" elements hardcoded, which does not take
into account longer pathnames that are possible in many filesystems.
Using "PATH_MAX" would also not be a solution because of portability
problems. Therefore, the best solution is to rely on the fact that
"getcwd (NULL, 0)" will "do the right thing" and return a
heap-allocated string containing the full path. With the new "getcwd"
module from gnulib, it is now possible to do that without worrying
about breaking some target.
With this patch "current_directory" is now the only place to check for
GDB's cwd.
gdb/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
* cli/cli-cmds.c (pwd_command): Use "getcwd (NULL, 0)".
(cd_command): Likewise. Free "current_directory" before
assigning to it.
* main.c (captured_main_1): Use "getcwd (NULL, 0)".
* mi/mi-cmd-env.c (mi_cmd_env_pwd): Likewise.
* top.c (gdb_dirbuf): Remove global declaration.
* top.h (gdb_dirbuf): Likewise.
---
gdb/cli/cli-cmds.c | 17 ++++++++++++-----
gdb/main.c | 5 ++---
gdb/mi/mi-cmd-env.c | 8 ++++----
gdb/top.c | 3 ---
gdb/top.h | 1 -
5 files changed, 18 insertions(+), 16 deletions(-)
Comments
On 09/19/2017 05:28 AM, Sergio Durigan Junior wrote:
> 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. Moreover, the way we're
> using "gdb_difbuf" along with "getcwd" is problematic because we
> declare the buffer with "1024" elements hardcoded, which does not take
> into account longer pathnames that are possible in many filesystems.
> Using "PATH_MAX" would also not be a solution because of portability
> problems. Therefore, the best solution is to rely on the fact that
> "getcwd (NULL, 0)" will "do the right thing" and return a
> heap-allocated string containing the full path. With the new "getcwd"
> module from gnulib, it is now possible to do that without worrying
> about breaking some target.
s/target/host/
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -382,13 +382,16 @@ pwd_command (char *args, int from_tty)
> {
> if (args)
> error (_("The \"pwd\" command does not take an argument: %s"), args);
> - if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
> +
> + gdb::unique_xmalloc_ptr<char> cwd (getcwd (NULL, 0));
> +
> + if (cwd.get () == NULL)
No need for get() here:
if (cwd == NULL)
> diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
> index 977b6e274d..1e4c09352f 100644
> --- a/gdb/mi/mi-cmd-env.c
> +++ b/gdb/mi/mi-cmd-env.c
> @@ -73,12 +73,12 @@ mi_cmd_env_pwd (const char *command, char **argv, int argc)
> }
>
> /* Otherwise the mi level is 2 or higher. */
> -
> - if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
> + gdb::unique_xmalloc_ptr<char> cwd (getcwd (NULL, 0));
> + if (cwd.get () == NULL)
Ditto.
Otherwise OK.
Thanks,
Pedro Alves
On Wednesday, September 20 2017, Pedro Alves wrote:
> On 09/19/2017 05:28 AM, Sergio Durigan Junior wrote:
>> 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. Moreover, the way we're
>> using "gdb_difbuf" along with "getcwd" is problematic because we
>> declare the buffer with "1024" elements hardcoded, which does not take
>> into account longer pathnames that are possible in many filesystems.
>> Using "PATH_MAX" would also not be a solution because of portability
>> problems. Therefore, the best solution is to rely on the fact that
>> "getcwd (NULL, 0)" will "do the right thing" and return a
>> heap-allocated string containing the full path. With the new "getcwd"
>> module from gnulib, it is now possible to do that without worrying
>> about breaking some target.
>
> s/target/host/
Fixed.
>> --- a/gdb/cli/cli-cmds.c
>> +++ b/gdb/cli/cli-cmds.c
>> @@ -382,13 +382,16 @@ pwd_command (char *args, int from_tty)
>> {
>> if (args)
>> error (_("The \"pwd\" command does not take an argument: %s"), args);
>> - if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
>> +
>> + gdb::unique_xmalloc_ptr<char> cwd (getcwd (NULL, 0));
>> +
>> + if (cwd.get () == NULL)
>
> No need for get() here:
>
> if (cwd == NULL)
Fixed.
>> diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
>> index 977b6e274d..1e4c09352f 100644
>> --- a/gdb/mi/mi-cmd-env.c
>> +++ b/gdb/mi/mi-cmd-env.c
>> @@ -73,12 +73,12 @@ mi_cmd_env_pwd (const char *command, char **argv, int argc)
>> }
>>
>> /* Otherwise the mi level is 2 or higher. */
>> -
>> - if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
>> + gdb::unique_xmalloc_ptr<char> cwd (getcwd (NULL, 0));
>> + if (cwd.get () == NULL)
>
> Ditto.
Fixed.
> Otherwise OK.
>
> Thanks,
> Pedro Alves
I'll wait until the other patches are approved to push this one, since
it depends on gnulib's getcwd.
Thanks,
@@ -382,13 +382,16 @@ pwd_command (char *args, int from_tty)
{
if (args)
error (_("The \"pwd\" command does not take an argument: %s"), args);
- if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
+
+ gdb::unique_xmalloc_ptr<char> cwd (getcwd (NULL, 0));
+
+ if (cwd.get () == NULL)
error (_("Error finding name of working directory: %s"),
safe_strerror (errno));
- if (strcmp (gdb_dirbuf, current_directory) != 0)
+ if (strcmp (cwd.get (), current_directory) != 0)
printf_unfiltered (_("Working directory %s\n (canonically %s).\n"),
- current_directory, gdb_dirbuf);
+ current_directory, cwd.get ());
else
printf_unfiltered (_("Working directory %s.\n"), current_directory);
}
@@ -416,7 +419,8 @@ cd_command (char *dir, int from_tty)
/* There's too much mess with DOSish names like "d:", "d:.",
"d:./foo" etc. Instead of having lots of special #ifdef'ed code,
simply get the canonicalized name of the current directory. */
- dir = getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
+ gdb::unique_xmalloc_ptr<char> cwd (getcwd (NULL, 0));
+ dir = cwd.get ();
#endif
len = strlen (dir);
@@ -434,7 +438,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]))
@@ -549,11 +549,10 @@ captured_main_1 (struct captured_main_args *context)
(xstrprintf ("%s: warning: ", gdb_program_name));
warning_pre_print = tmp_warn_preprint.get ();
- if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
+ current_directory = getcwd (NULL, 0);
+ if (current_directory == NULL)
perror_warning_with_name (_("error finding working directory"));
- current_directory = gdb_dirbuf;
-
/* Set the sysroot path. */
gdb_sysroot = relocate_gdb_directory (TARGET_SYSTEM_ROOT,
TARGET_SYSTEM_ROOT_RELOCATABLE);
@@ -73,12 +73,12 @@ mi_cmd_env_pwd (const char *command, char **argv, int argc)
}
/* Otherwise the mi level is 2 or higher. */
-
- if (! getcwd (gdb_dirbuf, sizeof (gdb_dirbuf)))
+ gdb::unique_xmalloc_ptr<char> cwd (getcwd (NULL, 0));
+ if (cwd.get () == NULL)
error (_("-environment-pwd: error finding name of working directory: %s"),
safe_strerror (errno));
-
- uiout->field_string ("cwd", gdb_dirbuf);
+
+ uiout->field_string ("cwd", cwd.get ());
}
/* Change working directory. */
@@ -133,9 +133,6 @@ show_confirm (struct ui_file *file, int from_tty,
char *current_directory;
-/* The directory name is actually stored here (usually). */
-char gdb_dirbuf[1024];
-
/* The last command line executed on the console. Used for command
repetitions. */
char *saved_command_line;
@@ -219,7 +219,6 @@ extern void ui_unregister_input_event_handler (struct ui *ui);
/* From top.c. */
extern char *saved_command_line;
extern int confirm;
-extern char gdb_dirbuf[1024];
extern int inhibit_gdbinit;
extern const char gdbinit[];