Re: [PATCH 5/7] PR gdb/15224 should "set history save on" by default

[Pedro Alves <palves@redhat.com>]

On 05/06/2013 10:48 AM, mbilal wrote:
> On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:
> 
>> (6) Change the default set history filename to ~/.gdb_history.
> 
> patch is here.
> 
> Index: top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/top.c,v
> retrieving revision 1.235
> diff -u -p -r1.235 top.c
> --- top.c    17 Apr 2013 01:02:02 -0000    1.235
> +++ top.c    6 May 2013 07:17:13 -0000
> @@ -1616,7 +1616,8 @@ void
>  init_history (void)
>  {
>    char *tmpenv;
> -
> +  char *homedir;
> +  homedir = getenv ("HOME");
>    tmpenv = getenv ("HISTSIZE");
>    if (tmpenv)
>      {
> @@ -1651,10 +1652,10 @@ init_history (void)
>           that was read.  */
>  #ifdef __MSDOS__
>        /* No leading dots in file names are allowed on MSDOS.  */
> -      history_filename = concat (current_directory, "/_gdb_history",
> +      history_filename = concat (homedir, "/_gdb_history",
>                   (char *)NULL);
>  #else
> -      history_filename = concat (current_directory, "/.gdb_history",
> +      history_filename = concat (homedir, "/.gdb_history",
>                   (char *)NULL);
>  #endif

This doesn't handle the case of HOME not being set.
It's better to use readline's tilde_expand, which handles that
by falling back to looking up the directory in the password
database.  If I'm reading the source correctly, I believe that's
also what bash does (likewise using readline's tilde_expand),
though I haven't stepped through it.  As simple as:

  #ifdef __MSDOS__
        /* No leading dots in file names are allowed on MSDOS.  */
        history_filename = tilde_expand ("~/_gdb_history"),
  #else
        history_filename = tilde_expand ("~/.gdb_history");
  #endif

>      }
> Index: testsuite/gdb.base/default.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/default.exp,v
> retrieving revision 1.64
> diff -u -p -r1.64 default.exp
> --- testsuite/gdb.base/default.exp    15 Apr 2013 18:09:02 -0000 1.64
> +++ testsuite/gdb.base/default.exp    6 May 2013 07:52:42 -0000
> @@ -642,7 +642,8 @@ gdb_test "show height" "Number of lines
>  #test show history expansion
>  gdb_test "show history expansion" "History expansion on command input is o\[a-z\]*.*" "show history expansion"
>  #test show history filename
> -gdb_test "show history filename" "The filename in which to record the command history is.*.gdb_history.*" "show history filename"
> +gdb_test "show history filename" "The filename in which to record the command history is .$env(HOME).*.gdb_history.*." \
> +    "show history filename .$env(HOME).*.gdb_history."

With remote host testing, dejagnu runs on different machine
(build) where GDB itself runs (host), so this assumption that $env(HOME)
always matches gdb's $HOME isn't always true.  I think we need to either
Use "remote_exec host" or gdb's "show environment" to get at the right
$HOME.

A ChangeLog entry is missing.

-- 
Pedro Alves