This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path
- From: Pedro Alves <palves at redhat dot com>
- To: mbilal <mbilal at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org, jan dot kratochvil at redhat dot com
- Date: Wed, 08 May 2013 16:46:52 +0100
- Subject: Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path
- References: <51877A32 dot 1030503 at codesourcery dot com> <51877A99 dot 4060503 at codesourcery dot com> <5188AA15 dot 5010904 at codesourcery dot com> <5188F70A dot 1030908 at codesourcery dot com>
On 05/07/2013 01:43 PM, mbilal wrote:
> please find updated patch .
>
> I made new 'set_history_filename' function to solve this problem
Please aim at posting self contained descriptions of patches, or
at least linking to them.
I happen to still recall the original issue, but I bet there are others
who don't. It's also very useful for future archaeology to find
the rationale for the patch close to the patch. Here's an example,
synthesized from <http://sourceware.org/ml/gdb-patches/2013-04/msg00066.html>:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
GDB resolves a relative .gdb_history path early on at init time, meaning,
the default history file written is the same that was
read, while "set history filename .gdb_history" resolves to
different history files at read and at write times if the user
changes directory in between, as seen in these examples:
$ unset GDBHISTFILE; gdb -ex "set history filename .gdb_history"
(gdb) show history filename
The filename in which to record the command history is "/tmp/.gdb_history".
(gdb) q
vs
$ cd /tmp
$ mkdir a
$ gdb -ex "set history filename .gdbhist"
(gdb) show history filename
The filename in which to record the command history is ".gdbhist".
(gdb) cd a
Working directory /tmp/a.
(gdb) q
$ cat a/.gdbhist
show history filename
cd a
q
$ ls .gdbhist a/.gdbhist
ls: cannot access .gdbhist: No such file or directory
a/.gdbhist
$
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Bonus points if that ends up in the commit log.
IIRC, a previous version had a test; what happened to it?
Did it cover the issue this patch proposes fixing?
> diff --git a/gdb/top.c b/gdb/top.c
> index 480b67e..20aecc9 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -48,6 +48,7 @@
> #include "interps.h"
> #include "observer.h"
> #include "maint.h"
> +#include "filenames.h"
>
> /* readline include files. */
> #include "readline/readline.h"
> @@ -1607,6 +1608,14 @@ set_verbose (char *args, int from_tty, struct cmd_list_element *c)
> }
> }
>
> +static void
> +set_history_filename (char *args, int from_tty, struct cmd_list_element *c)
> +{
> + if (!IS_ABSOLUTE_PATH (*(char **) c->var))
> + *(char **) c->var = concat (current_directory, "/", *(char **) c->var,
> + (char *)NULL);
> +}
Missing space before NULL. That's a lot of casting. We can just refer
to history_filename directly. It'd be very good to have a comment here with the
rationale for this -- there's one in init_history we can reuse. So:
if (!IS_ABSOLUTE_PATH (history_filename))
{
/* We include the current directory so that if the user changes
directories the file written will be the same as the one
that was read. */
history_filename = concat (current_directory, "/", history_filename,
(char *) NULL);
}
Also, a ChangeLog entry is missing.
Thanks,
--
Pedro Alves