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

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

[mbilal]

Hi,

Here is the proposed patch series of 'set history save on' by default
A thread has been discussed here 
http://sourceware.org/ml/gdb-patches/2013-04/msg00067.html




Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.15497
diff -u -p -r1.15497 ChangeLog
--- ChangeLog    1 May 2013 14:54:18 -0000    1.15497
+++ ChangeLog    6 May 2013 09:33:22 -0000
@@ -1,3 +1,16 @@
+2013-05-06  Muhammad Bilal  <mbilal@codesourcery.com>
+
+    PR gdb/15224
+    * top.c (init_main): Modified write_history_p.
+    (quit_force): Add condition input_from_terminal_p.
+    (command_line_input): Unify interactivity tests to
+    use input_from_terminal_p.
+
+    * cli/cli-setshow.c (do_set_command): set history filaname
+    with absolute patch.
+
+    * NEWS: Updated.
+


Thanks,
-Bilal

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

[mbilal]

On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:

 > (1) Fix relative "set history filename" to by immediately converted 
to absolute
     form

Here is a proposed patch with testcase.



Index: cli/cli-setshow.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-setshow.c,v
retrieving revision 1.61
diff -u -p -r1.61 cli-setshow.c
--- cli/cli-setshow.c    10 Apr 2013 15:11:12 -0000    1.61
+++ cli/cli-setshow.c    6 May 2013 05:23:46 -0000
@@ -22,6 +22,7 @@
  #include "gdb_string.h"
  #include "arch-utils.h"
  #include "observer.h"
+#include "filenames.h"

  #include "ui-out.h"

@@ -247,6 +248,13 @@ do_set_command (char *arg, int from_tty,
      if (*(char **) c->var == NULL
          || strcmp (*(char **) c->var, val) != 0)
        {
+      if (strcmp (c->name, "filename") == 0)
+        {
+        if (!IS_ABSOLUTE_PATH (val))
+          val = concat (current_directory, "/", val,
+          (char *)NULL);
+        }
+
          xfree (*(char **) c->var);
          *(char **) c->var = val;
Index: gdb.base/setshow.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/setshow.exp,v
retrieving revision 1.30
diff -u -p -r1.30 setshow.exp
--- gdb.base/setshow.exp    24 Apr 2013 14:29:17 -0000    1.30
+++ gdb.base/setshow.exp    6 May 2013 06:01:05 -0000
@@ -173,7 +173,8 @@ gdb_test "show history expansion on" "Hi
  gdb_test_no_output "set history filename foobar.baz" \
      "set history filename foobar.baz"
  #test show history filename foobar.baz
-gdb_test "show history filename" "The filename in which to record the 
command history is \"foobar.baz\"..*"  "show history filename (foobar.baz)"
+gdb_test "show history filename" "The filename in which to record the 
command history is \"[file join [pwd] foobar.baz]\"..*" \
+    "show history filename \"[file join [pwd] foobar.baz]\"."
  #test set history save on
  gdb_test_no_output "set history save on" "set history save on"
  #test show history save on





thanks,
-Bilal

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

[mbilal]

On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:

 > (2) Unify interactivity tests to use input_from_terminal_p:

patch for this

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 06:12:07 -0000
@@ -1041,7 +1041,7 @@ command_line_input (char *prompt_arg, in

    /* Do history expansion if that is wished.  */
    if (history_expansion_p && instream == stdin
-      && ISATTY (instream))
+      && ISATTY (instream) && input_from_terminal_p())
      {
        char *history_value;
        int expanded;


Thanks,
-Bilal

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

[mbilal]

On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:

 > (3) Fix currently incorrectly touched history file even if no 
commands get

here is patch

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 06:16:04 -0000
@@ -1430,7 +1430,8 @@ quit_force (char *args, int from_tty)
    /* Save the history information if it is appropriate to do so. */
    DO_TRY
      {
-      if (write_history_p && history_filename)
+      if (write_history_p && history_filename
+          && input_from_terminal_p())
      write_history (history_filename);
      }
    DO_PRINT_EX;



I have tested

$ ls -als ~/gdb_history
0 -rw-rw-r-- 1 mbilal mbilal 0 May  6 13:12 /home/mbilal/gdb_history
$ date
Mon May  6 13:16:09 PKT 2013
$ ./gdb-cvs/src/gdb/gdb gdb -ex "set history filename ~/gdb_history" < 
/dev/null
GNU gdb (GDB) 7.6.50.20130503-cvs
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /usr/local/bin/gdb...done.
(gdb) quit
$ ls -als ~/gdb_history
0 -rw-rw-r-- 1 mbilal mbilal 0 May  6 13:12 /home/mbilal/gdb_history





Thanks,
-Bilal

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

[mbilal]

On 05/06/2013 02:43 PM, mbilal wrote:
> On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:
>
> > (3) Fix currently incorrectly touched history file even if no 
> commands get
>
> here is patch
>
> 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 06:16:04 -0000
> @@ -1430,7 +1430,8 @@ quit_force (char *args, int from_tty)
>    /* Save the history information if it is appropriate to do so. */
>    DO_TRY
>      {
> -      if (write_history_p && history_filename)
> +      if (write_history_p && history_filename
> +          && input_from_terminal_p())
>      write_history (history_filename);
>      }
>    DO_PRINT_EX;
>
>
>
> I have tested
>
> $ ls -als ~/gdb_history
> 0 -rw-rw-r-- 1 mbilal mbilal 0 May  6 13:12 /home/mbilal/gdb_history
> $ date
> Mon May  6 13:16:09 PKT 2013
> $ ./gdb-cvs/src/gdb/gdb gdb -ex "set history filename ~/gdb_history" < 
> /dev/null
> GNU gdb (GDB) 7.6.50.20130503-cvs
> Copyright (C) 2013 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later 
> <http://gnu.org/licenses/gpl.html>
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.  Type "show 
> copying"
> and "show warranty" for details.
> This GDB was configured as "i686-pc-linux-gnu".
> Type "show configuration" for configuration details.
> For bug reporting instructions, please see:
> <http://www.gnu.org/software/gdb/bugs/>...
> Reading symbols from /usr/local/bin/gdb...done.
> (gdb) quit
> $ ls -als ~/gdb_history
> 0 -rw-rw-r-- 1 mbilal mbilal 0 May  6 13:12 /home/mbilal/gdb_history
>
>
>
>
>
> Thanks,
> -Bilal
ping?

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

[mbilal]

On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:

 > (4) Modify GDB testsuite to always disable history saving.

patch for this.

Index: lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.230
diff -u -p -r1.230 gdb.exp
--- lib/gdb.exp    15 Apr 2013 20:37:37 -0000    1.230
+++ lib/gdb.exp    6 May 2013 07:01:55 -0000
@@ -58,7 +58,7 @@ set BUILD_DATA_DIRECTORY "[pwd]/../data-
  # INTERNAL_GDBFLAGS contains flags that the testsuite requires.
  global INTERNAL_GDBFLAGS
  if ![info exists INTERNAL_GDBFLAGS] {
-    set INTERNAL_GDBFLAGS "-nw -nx -data-directory $BUILD_DATA_DIRECTORY"
+    set INTERNAL_GDBFLAGS "-nw -nx  -ex set\\ history\\ save\\ off 
-data-directory $BUILD_DATA_DIRECTORY"
  }

  # The variable gdb_prompt is a regexp which matches the gdb prompt.
Index: 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
--- gdb.base/default.exp    15 Apr 2013 18:09:02 -0000    1.64
+++ gdb.base/default.exp    6 May 2013 07:02:53 -0000
@@ -521,8 +521,9 @@ gdb_test "set height" "Argument required
  gdb_test_no_output "set history expansion" "set history expansion"
  #test set history filename
  gdb_test "set history filename" "Argument required .filename to set it 
to.*" "set history filename"
+#We are always disable history saving on testsuite
  #test set history save
-gdb_test_no_output "set history save" "set history save"
+#gdb_test_no_output "set history save" "set history save"
  #test set history size
  gdb_test "set history size" "Argument required .integer to set it 
to.*" "set history size"
  #test set history
@@ -644,7 +645,7 @@ gdb_test "show history expansion" "Histo
  #test show history filename
  gdb_test "show history filename" "The filename in which to record the 
command history is.*.gdb_history.*" "show history filename"
  #test show history save
-gdb_test "show history save" "Saving of the history record on exit is 
on." "show history save"
+gdb_test "show history save" "Saving of the history record on exit is 
off." "show history save"
  #test show history size
  gdb_test "show history size" "The size of the command history is.*" 
"show history size"
  #test show history
Index: gdb.base/setshow.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/setshow.exp,v
retrieving revision 1.30
diff -u -p -r1.30 setshow.exp
--- gdb.base/setshow.exp    24 Apr 2013 14:29:17 -0000    1.30
+++ gdb.base/setshow.exp    6 May 2013 07:08:29 -0000
@@ -174,10 +174,11 @@ gdb_test_no_output "set history filename
      "set history filename foobar.baz"
  #test show history filename foobar.baz
  gdb_test "show history filename" "The filename in which to record the 
command history is \"foobar.baz\"..*"  "show history filename (foobar.baz)"
+#We are always disable history saving on testsuite
  #test set history save on
-gdb_test_no_output "set history save on" "set history save on"
+#gdb_test_no_output "set history save on" "set history save on"
  #test show history save on
-gdb_test "show history save" "Saving of the history record on exit is 
on..*"  "show history save (on)"
+gdb_test "show history save" "Saving of the history record on exit is 
off..*"  "show history save (off)"
  #test set history size 100
  gdb_test_no_output "set history size 100" "set history size 100"
  #test show history size 100

Thanks,
-Bilal

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

[Abid, Hafiz]

On 06/05/13 10:47:21, mbilal wrote:
> On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:
> 
> > (4) Modify GDB testsuite to always disable history saving.
> 
> patch for this.
> 
> Index: lib/gdb.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
> retrieving revision 1.230
> diff -u -p -r1.230 gdb.exp
> --- lib/gdb.exp    15 Apr 2013 20:37:37 -0000    1.230
> +++ lib/gdb.exp    6 May 2013 07:01:55 -0000
> @@ -58,7 +58,7 @@ set BUILD_DATA_DIRECTORY "[pwd]/../data-
>  # INTERNAL_GDBFLAGS contains flags that the testsuite requires.
>  global INTERNAL_GDBFLAGS
>  if ![info exists INTERNAL_GDBFLAGS] {
> -    set INTERNAL_GDBFLAGS "-nw -nx -data-directory  
> $BUILD_DATA_DIRECTORY"
> +    set INTERNAL_GDBFLAGS "-nw -nx  -ex set\\ history\\ save\\ off  
> -data-directory $BUILD_DATA_DIRECTORY"
>  }
> 
>  # The variable gdb_prompt is a regexp which matches the gdb prompt.
> Index: 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
> --- gdb.base/default.exp    15 Apr 2013 18:09:02 -0000    1.64
> +++ gdb.base/default.exp    6 May 2013 07:02:53 -0000
> @@ -521,8 +521,9 @@ gdb_test "set height" "Argument required
>  gdb_test_no_output "set history expansion" "set history expansion"
>  #test set history filename
>  gdb_test "set history filename" "Argument required .filename to set  
> it to.*" "set history filename"
> +#We are always disable history saving on testsuite
s/are always disable/always disable/

>  #test set history save
> -gdb_test_no_output "set history save" "set history save"
> +#gdb_test_no_output "set history save" "set history save"
>  #test set history size
>  gdb_test "set history size" "Argument required .integer to set it  
> to.*" "set history size"
>  #test set history
> @@ -644,7 +645,7 @@ gdb_test "show history expansion" "Histo
>  #test show history filename
>  gdb_test "show history filename" "The filename in which to record  
> the command history is.*.gdb_history.*" "show history filename"
>  #test show history save
> -gdb_test "show history save" "Saving of the history record on exit  
> is on." "show history save"
> +gdb_test "show history save" "Saving of the history record on exit  
> is off." "show history save"
>  #test show history size
>  gdb_test "show history size" "The size of the command history is.*"  
> "show history size"
>  #test show history
> Index: gdb.base/setshow.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/setshow.exp,v
> retrieving revision 1.30
> diff -u -p -r1.30 setshow.exp
> --- gdb.base/setshow.exp    24 Apr 2013 14:29:17 -0000    1.30
> +++ gdb.base/setshow.exp    6 May 2013 07:08:29 -0000
> @@ -174,10 +174,11 @@ gdb_test_no_output "set history filename
>      "set history filename foobar.baz"
>  #test show history filename foobar.baz
>  gdb_test "show history filename" "The filename in which to record  
> the command history is \"foobar.baz\"..*"  "show history filename  
> (foobar.baz)"
> +#We are always disable history saving on testsuite
Same here.

>  #test set history save on
> -gdb_test_no_output "set history save on" "set history save on"
> +#gdb_test_no_output "set history save on" "set history save on"
>  #test show history save on
> -gdb_test "show history save" "Saving of the history record on exit  
> is on..*"  "show history save (on)"
> +gdb_test "show history save" "Saving of the history record on exit  
> is off..*"  "show history save (off)"
>  #test set history size 100
>  gdb_test_no_output "set history size 100" "set history size 100"
>  #test show history size 100
> 
> Thanks,
> -Bilal

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

[Pedro Alves]

On 05/06/2013 10:47 AM, mbilal wrote:
> On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:
> 
>> (4) Modify GDB testsuite to always disable history saving.
> 
> patch for this.
> 
> Index: lib/gdb.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
> retrieving revision 1.230
> diff -u -p -r1.230 gdb.exp
> --- lib/gdb.exp    15 Apr 2013 20:37:37 -0000    1.230
> +++ lib/gdb.exp    6 May 2013 07:01:55 -0000
> @@ -58,7 +58,7 @@ set BUILD_DATA_DIRECTORY "[pwd]/../data-
>  # INTERNAL_GDBFLAGS contains flags that the testsuite requires.
>  global INTERNAL_GDBFLAGS
>  if ![info exists INTERNAL_GDBFLAGS] {
> -    set INTERNAL_GDBFLAGS "-nw -nx -data-directory $BUILD_DATA_DIRECTORY"
> +    set INTERNAL_GDBFLAGS "-nw -nx  -ex set\\ history\\ save\\ off -data-directory $BUILD_DATA_DIRECTORY"

 spurious space?               ------^^


>  }
> 
>  # The variable gdb_prompt is a regexp which matches the gdb prompt.
> Index: 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
> --- gdb.base/default.exp    15 Apr 2013 18:09:02 -0000    1.64
> +++ gdb.base/default.exp    6 May 2013 07:02:53 -0000
> @@ -521,8 +521,9 @@ gdb_test "set height" "Argument required
>  gdb_test_no_output "set history expansion" "set history expansion"
>  #test set history filename
>  gdb_test "set history filename" "Argument required .filename to set it to.*" "set history filename"
> +#We are always disable history saving on testsuite
>  #test set history save
> -gdb_test_no_output "set history save" "set history save"
> +#gdb_test_no_output "set history save" "set history save"
>  #test set history size

I don't understand this.  Why disable the test?

>  gdb_test "set history size" "Argument required .integer to set it to.*" "set history size"
>  #test set history
> @@ -644,7 +645,7 @@ gdb_test "show history expansion" "Histo
>  #test show history filename
>  gdb_test "show history filename" "The filename in which to record the command history is.*.gdb_history.*" "show history filename"
>  #test show history save
> -gdb_test "show history save" "Saving of the history record on exit is on." "show history save"
> +gdb_test "show history save" "Saving of the history record on exit is off." "show history save"
>  #test show history size
>  gdb_test "show history size" "The size of the command history is.*" "show history size"
>  #test show history
> Index: gdb.base/setshow.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/setshow.exp,v
> retrieving revision 1.30
> diff -u -p -r1.30 setshow.exp
> --- gdb.base/setshow.exp    24 Apr 2013 14:29:17 -0000    1.30
> +++ gdb.base/setshow.exp    6 May 2013 07:08:29 -0000
> @@ -174,10 +174,11 @@ gdb_test_no_output "set history filename
>      "set history filename foobar.baz"
>  #test show history filename foobar.baz
>  gdb_test "show history filename" "The filename in which to record the command history is \"foobar.baz\"..*"  "show history filename (foobar.baz)"
> +#We are always disable history saving on testsuite
>  #test set history save on
> -gdb_test_no_output "set history save on" "set history save on"
> +#gdb_test_no_output "set history save on" "set history save on"
>  #test show history save on
> -gdb_test "show history save" "Saving of the history record on exit is on..*"  "show history save (on)"
> +gdb_test "show history save" "Saving of the history record on exit is off..*"  "show history save (off)"
>  #test set history size 100
>  gdb_test_no_output "set history size 100" "set history size 100"
>  #test show history size 100

Likewise.

-- 
Pedro Alves

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

[mbilal]

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
      }
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."
  #test show history save
  gdb_test "show history save" "Saving of the history record on exit is 
on." "show history save"
  #test show history size


Thanks,
-Bilal

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

[mbilal]

On 05/06/2013 02:48 PM, 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
>      }
> 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."
>  #test show history save
>  gdb_test "show history save" "Saving of the history record on exit is 
> on." "show history save"
>  #test show history size
>
>
> Thanks,
> -Bilal
ping

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[mbilal]

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

Hi
Please find attached updated patch.

In This patch Default location of History file has been changed to user 
home directory (~/.gdb_history).





2013-05-15  Muhammad Bilal  <mbilal@codesourcery.com>

     PR gdb/15224
     * top.c (init_history): Change the default history filename
     to ~/.gdb_history.

     * NEWS: Updated.

2013-05-15  Muhammad Bilal  <mbilal@codesourcery.com>

     PR gdb/15224
     * gdb.base/default.exp: Test default history filename
     Path.

2013-05-15  Muhammad Bilal  <mbilal@codesourcery.com>

     PR gdb/15224
     * gdb.texinfo (Command History): Change the default history filename
     to ~/.gdb_history.




Thanks,
-Bilal




[-- Attachment #2: 6 --]
[-- Type: text/plain, Size: 2387 bytes --]

diff --git a/gdb/top.c b/gdb/top.c
index 480b67e..83a5fee 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1651,11 +1651,9 @@ 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",
-                                (char *)NULL);
+      history_filename = tilde_expand ("~/_gdb_history");
 #else
-      history_filename = concat (current_directory, "/.gdb_history",
-                                (char *)NULL);
+      history_filename = tilde_expand ("~/.gdb_history");
 #endif
     }
   read_history (history_filename);
diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
index 9230ee8..9d2e9ce 100644
--- a/gdb/testsuite/gdb.base/default.exp
+++ b/gdb/testsuite/gdb.base/default.exp
@@ -800,6 +800,15 @@ if ![istarget "*-*-udi*"] then {
        timeout         { fail "(timeout) target remote" }
     }
 }
+#get home directory path
+gdb_test_multiple "show environment HOME" "show home directory" {
+    -re "\nHOME = (.*).\n.*" {
+       set HOME $expect_out(1,string)    
+    }
+}
+#test show history filename
+gdb_test "show history filename" "The filename in which to record the command history is \"[file join $HOME .gdb_history]\"..*" \
+    "show history filename ([file join $HOME foobar.baz])"
 
 #test target
 gdb_test "target" "Argument required .target name.*" "target"
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1869d74..a9a1f89 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21455,7 +21455,7 @@ list, and where it writes the command history from this session when it
 exits.  You can access this list through history expansion or through
 the history command editing characters listed below.  This file defaults
 to the value of the environment variable @code{GDBHISTFILE}, or to
-@file{./.gdb_history} (@file{./_gdb_history} on MS-DOS) if this variable
+@file{~/.gdb_history} (@file{~/_gdb_history} on MS-DOS) if this variable
 is not set.
diff --git a/gdb/NEWS b/gdb/NEWS
index 7cd1646..04b929f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,9 @@
 
 *** Changes since GDB 7.6
 
+* Default location of History file has been changed to user 
+home directory (~/.gdb_history).
+
 * Python scripting
 
   ** Frame filters and frame decorators have been added.

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Eli Zaretskii]

> Date: Wed, 15 May 2013 16:08:31 +0500
> From: mbilal <mbilal@codesourcery.com>
> CC: <jan.kratochvil@redhat.com>
> 
> In This patch Default location of History file has been changed to user 
> home directory (~/.gdb_history).

Any reasons why we should be changing the default?  Is it bad to have
a different history file in each project's directory?

The patches for the documentation parts are approved.

Thanks.

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[mbilal]

On Wednesday, May 15, 2013 4:31 PM Eli wrote:

>Any reasons why we  should be changing the default?  Is it bad to have
 >a different history file in each project's directory?

On Mon, 01 Apr 2013 14:29:30 +0200, Jan Kratochvil wrote:
> I see there should be also changed the default to:
>       set history filename ~/.gdb_history

>Otherwise it is a similar pain to ./.gdbinit, it is also non-standard to any
>other normal application which use history files in $HOME, I do not find using
>./.gdb_history by default as acceptable.
All therequirement for this issue has beendiscussed here.  

  http://sourceware.org/ml/gdb-patches/2013-04/msg00067.html


Thanks,
-Bilal

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Eli Zaretskii]

> Date: Wed, 15 May 2013 17:07:35 +0500
> From: mbilal <mbilal@codesourcery.com>
> CC: <palves@redhat.com>, <gdb-patches@sourceware.org>,
> 	<jan.kratochvil@redhat.com>
> 
> On Wednesday, May 15, 2013 4:31 PM Eli wrote:
> 
> >Any reasons why we  should be changing the default?  Is it bad to have
>  >a different history file in each project's directory?
> 
> On Mon, 01 Apr 2013 14:29:30 +0200, Jan Kratochvil wrote:
> > I see there should be also changed the default to:
> >       set history filename ~/.gdb_history
> 
> >Otherwise it is a similar pain to ./.gdbinit, it is also non-standard to any
> >other normal application which use history files in $HOME, I do not find using
> >./.gdb_history by default as acceptable.
> All therequirement for this issue has beendiscussed here.  
> 
>   http://sourceware.org/ml/gdb-patches/2013-04/msg00067.html

Sigh...  Another gratuitous backward incompatibility creeps in.

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Pedro Alves]

On 05/15/2013 02:26 PM, Eli Zaretskii wrote:
>> Date: Wed, 15 May 2013 17:07:35 +0500
>> From: mbilal <mbilal@codesourcery.com>
>> CC: <palves@redhat.com>, <gdb-patches@sourceware.org>,
>> 	<jan.kratochvil@redhat.com>
>>
>> On Wednesday, May 15, 2013 4:31 PM Eli wrote:
>>
>>> Any reasons why we  should be changing the default?  Is it bad to have
>>  >a different history file in each project's directory?
>>
>> On Mon, 01 Apr 2013 14:29:30 +0200, Jan Kratochvil wrote:
>>> I see there should be also changed the default to:
>>>       set history filename ~/.gdb_history
>>
>>> Otherwise it is a similar pain to ./.gdbinit, it is also non-standard to any
>>> other normal application which use history files in $HOME, I do not find using
>>> ./.gdb_history by default as acceptable.
>> All therequirement for this issue has beendiscussed here.  
>>
>>   http://sourceware.org/ml/gdb-patches/2013-04/msg00067.html
> 
> Sigh...  Another gratuitous backward incompatibility creeps in.

Keep in mind that history saving isn't enabled by default.

Would you prefer if we did something like:

At startup time, if "set history filename" hasn't been
used (tracked with a new global flag), then check whether there's
a $cwd/.gdb_history file, and if so, output a warning stating that
it's no longer read by default.  The warning could also
suggest adding "set history filename .gdb_history" to .gdbinit
to get back the old behavior.

Thus if the user already has "set history filename ~/.gdb_history"
in .gdbinit she'll not see any warning.

But I'm really not sure it's worth the bother though.

-- 
Pedro Alves

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Eli Zaretskii]

> Date: Wed, 22 May 2013 18:24:13 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: mbilal <mbilal@codesourcery.com>, gdb-patches@sourceware.org,
>         jan.kratochvil@redhat.com
> 
> At startup time, if "set history filename" hasn't been
> used (tracked with a new global flag), then check whether there's
> a $cwd/.gdb_history file, and if so, output a warning stating that
> it's no longer read by default.  The warning could also
> suggest adding "set history filename .gdb_history" to .gdbinit
> to get back the old behavior.

That's even more annoying, IMO.

I think gratuitous backward incompatibility is evil, especially in
programs, like GDB, where there are no good facilities for
version-specific scripting.  We are breaking scripts out there for no
good reason, and we aren't giving the users of those scripts _any_
means to get their previous behavior cleanly.

But I know I'm somehow in the minority here.  It's probably the age or
something.

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Pedro Alves]

On 05/22/2013 06:49 PM, Eli Zaretskii wrote:
>> Date: Wed, 22 May 2013 18:24:13 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: mbilal <mbilal@codesourcery.com>, gdb-patches@sourceware.org,
>>         jan.kratochvil@redhat.com
>>
>> At startup time, if "set history filename" hasn't been
>> used (tracked with a new global flag), then check whether there's
>> a $cwd/.gdb_history file, and if so, output a warning stating that
>> it's no longer read by default.  The warning could also
>> suggest adding "set history filename .gdb_history" to .gdbinit
>> to get back the old behavior.
> 
> That's even more annoying, IMO.
> 
> I think gratuitous backward incompatibility is evil, especially in
> programs, like GDB, where there are no good facilities for
> version-specific scripting.  We are breaking scripts out there for no
> good reason, and we aren't giving the users of those scripts _any_
> means to get their previous behavior cleanly.

I understand the sentiment, but I'd rather not generalize, and look
at it on a case by case basis.  Scripts do have a means get the
previous behavior.  Simply adding this to .gdbinit:

 "set history filename .gdb_history"

get you the old behavior back.  And works the same with the
older GDBs too.  Note the user must already have added

 "set history save on"

to .gdbinit in previous releases to have history saving.
He'll just need to add one more line next to that.

If you're talking about getting the previous behavior of
not having history enabled, then the user can just add

 "set history save off"

to .gdbinit.

> But I know I'm somehow in the minority here.  It's probably the age or
> something.

:-)

The reasoning for changing the default is that we (Pedro/Jan/Muhammad)
believe enabling history by default is a better default that having it
disabled by default, as currently.  Couple the fact that ".gdb_history"
is a dot/hidden file, with enabling history saving by default, and users
could end up with their filesystem littered with random hidden .gdb_history
files.  I think GDB shouldn't do that by default.  So in order to enable
history saving by default, we believe we should default to ~/.gdb_history
instead first.  bash also defaults to saving history under $HOME, and I'd
think most other interactive programs/shell do so too.  So it feels like
GDB is the odd one out here.

-- 
Pedro Alves

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Eli Zaretskii]

> Date: Wed, 22 May 2013 19:08:31 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: mbilal@codesourcery.com, gdb-patches@sourceware.org,
>         jan.kratochvil@redhat.com
> 
> I understand the sentiment, but I'd rather not generalize, and look
> at it on a case by case basis.

Since my opinions are being voted down "on a case by case basis", that
doesn't help me.  I thought maybe generalizing will, because all these
incompatibilities add up to a tendency that I think is dead wrong.

> Scripts do have a means get the previous behavior.  Simply adding
> this to .gdbinit:

But .gdbinit in the current directory is no longer read by default, so
I can't, not without restoring the old behavior, which does involve
using a command that will cause old GDB's to barf.

> The reasoning for changing the default is that we (Pedro/Jan/Muhammad)
> believe enabling history by default is a better default that having it
> disabled by default, as currently.

Another gratuitous incompatibility.

> Couple the fact that ".gdb_history"
> is a dot/hidden file, with enabling history saving by default, and users
> could end up with their filesystem littered with random hidden .gdb_history
> files.  I think GDB shouldn't do that by default.  So in order to enable
> history saving by default, we believe we should default to ~/.gdb_history
> instead first.  bash also defaults to saving history under $HOME, and I'd
> think most other interactive programs/shell do so too.  So it feels like
> GDB is the odd one out here.

Being the odd one out is not a reason good enough to change behavior
that was the default for a long time.

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Pedro Alves]

On 05/22/2013 08:18 PM, Eli Zaretskii wrote:
>> Scripts do have a means get the previous behavior.  Simply adding
>> this to .gdbinit:
> 
> But .gdbinit in the current directory is no longer read by default, so
> I can't, not without restoring the old behavior, which does involve
> using a command that will cause old GDB's to barf.
> 
>> The reasoning for changing the default is that we (Pedro/Jan/Muhammad)
>> believe enabling history by default is a better default that having it
>> disabled by default, as currently.
> 
> Another gratuitous incompatibility.

Sorry, but simply calling changes gratuitous when I've made an
effort to explain why I believe they're good doesn't help.  :-/

The idea is that enabling the feature by default will expose
it to more users, who will benefit from it, most (educated-guessing
here, of course) not being aware GDB presently can already use
history from previous sessions.

> 
>> Couple the fact that ".gdb_history"
>> is a dot/hidden file, with enabling history saving by default, and users
>> could end up with their filesystem littered with random hidden .gdb_history
>> files.  I think GDB shouldn't do that by default.  So in order to enable
>> history saving by default, we believe we should default to ~/.gdb_history
>> instead first.  bash also defaults to saving history under $HOME, and I'd
>> think most other interactive programs/shell do so too.  So it feels like
>> GDB is the odd one out here.
> 
> Being the odd one out is not a reason good enough to change behavior
> that was the default for a long time.

The default has been to not save the history at all.

When weighing the pros and cons, I believe the pros outweigh the cons.

That's just my opinion, and I've just tried to clarify why I have it.

-- 
Pedro Alves

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Muhammad Bilal]

On 05/23/2013 12:39 AM, Pedro Alves wrote:
> On 05/22/2013 08:18 PM, Eli Zaretskii wrote:
>>> Scripts do have a means get the previous behavior.  Simply adding
>>> this to .gdbinit:
>> But .gdbinit in the current directory is no longer read by default, so
>> I can't, not without restoring the old behavior, which does involve
>> using a command that will cause old GDB's to barf.
>>
>>> The reasoning for changing the default is that we (Pedro/Jan/Muhammad)
>>> believe enabling history by default is a better default that having it
>>> disabled by default, as currently.
>> Another gratuitous incompatibility.
> Sorry, but simply calling changes gratuitous when I've made an
> effort to explain why I believe they're good doesn't help.  :-/
>
> The idea is that enabling the feature by default will expose
> it to more users, who will benefit from it, most (educated-guessing
> here, of course) not being aware GDB presently can already use
> history from previous sessions.
>
>>> Couple the fact that ".gdb_history"
>>> is a dot/hidden file, with enabling history saving by default, and users
>>> could end up with their filesystem littered with random hidden .gdb_history
>>> files.  I think GDB shouldn't do that by default.  So in order to enable
>>> history saving by default, we believe we should default to ~/.gdb_history
>>> instead first.  bash also defaults to saving history under $HOME, and I'd
>>> think most other interactive programs/shell do so too.  So it feels like
>>> GDB is the odd one out here.
>> Being the odd one out is not a reason good enough to change behavior
>> that was the default for a long time.
> The default has been to not save the history at all.
>
> When weighing the pros and cons, I believe the pros outweigh the cons.
>
> That's just my opinion, and I've just tried to clarify why I have it.
>
ping?

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> The default has been to not save the history at all.
Pedro> When weighing the pros and cons, I believe the pros outweigh the cons.
Pedro> That's just my opinion, and I've just tried to clarify why I have it.

I've re-read this thread and I agree that the change should be made.

thanks,
Tom

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[Doug Evans]

On Thu, Aug 1, 2013 at 8:41 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> The default has been to not save the history at all.
> Pedro> When weighing the pros and cons, I believe the pros outweigh the cons.
> Pedro> That's just my opinion, and I've just tried to clarify why I have it.
>
> I've re-read this thread and I agree that the change should be made.

I concur.

Re: [PATCH 5/7] PR gdb/15224 , Change the default set history filename to ~/.gdb_history.

[mbilal]

On 05/15/2013 04:08 PM, mbilal wrote:
> Hi
> Please find attached updated patch.
>
> In This patch Default location of History file has been changed to 
> user home directory (~/.gdb_history).
>
>
>
>
>
> 2013-05-15  Muhammad Bilal  <mbilal@codesourcery.com>
>
>     PR gdb/15224
>     * top.c (init_history): Change the default history filename
>     to ~/.gdb_history.
>
>     * NEWS: Updated.
>
> 2013-05-15  Muhammad Bilal  <mbilal@codesourcery.com>
>
>     PR gdb/15224
>     * gdb.base/default.exp: Test default history filename
>     Path.
>
> 2013-05-15  Muhammad Bilal  <mbilal@codesourcery.com>
>
>     PR gdb/15224
>     * gdb.texinfo (Command History): Change the default history filename
>     to ~/.gdb_history.
>
>
>
>
> Thanks,
> -Bilal
>
>
>
ping?

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

[Pedro Alves]

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

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

[mbilal]

On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:


 > (7) Enable "set history save on" by default as you already did in 
your patch.

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:58:47 -0000
@@ -1715,7 +1715,7 @@ init_main (void)
    /* Set the important stuff up for command editing.  */
    command_editing_p = 1;
    history_expansion_p = 0;
-  write_history_p = 0;
+  write_history_p = 1;

    /* Setup important stuff for command line editing.  */
    rl_completion_word_break_hook = gdb_completion_word_break_characters;

Thanks,
-Bilal

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

[mbilal]

Here is doc and news entry.

Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.1084
diff -u -p -r1.1084 gdb.texinfo
--- doc/gdb.texinfo    29 Apr 2013 17:32:43 -0000    1.1084
+++ doc/gdb.texinfo    6 May 2013 09:28:27 -0000
@@ -21286,7 +21286,7 @@ is not set.
  @item set history save
  @itemx set history save on
  Record command history in a file, whose name may be specified with the
-@code{set history filename} command.  By default, this option is disabled.
+@code{set history filename} command.  By default, this option is enabled.

  @item set history save off
  Stop recording command history in a file.
Index: doc/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
retrieving revision 1.1452
diff -u -p -r1.1452 ChangeLog
--- doc/ChangeLog    29 Apr 2013 17:32:43 -0000    1.1452
+++ doc/ChangeLog    6 May 2013 09:28:56 -0000
@@ -1,3 +1,8 @@
+2013-05-06  Muhammad Bilal  <mbilal@codesourcery.com>
+
+    PR gdb/15224:
+    * gdb.texinfo (Command history): Set history save enabled.
+
  2013-04-29  Tom Tromey  <tromey@redhat.com>

      PR python/14204:
Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.593
diff -u -p -r1.593 NEWS
--- NEWS    16 Apr 2013 14:36:53 -0000    1.593
+++ NEWS    6 May 2013 09:30:13 -0000
@@ -3,6 +3,9 @@

  *** Changes since GDB 7.6

+* The command 'history save' is now default enabled.
+Disabled 'history save' on testsuiltes.
+
  * New commands:
  catch rethrow
    Like "catch throw", but catches a re-thrown exception.


Thanks,
-Bilal

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

[Abid, Hafiz]

On 06/05/13 10:50:59, mbilal wrote:
> Here is doc and news entry.
> 
> Index: doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.1084
> diff -u -p -r1.1084 gdb.texinfo
> --- doc/gdb.texinfo    29 Apr 2013 17:32:43 -0000    1.1084
> +++ doc/gdb.texinfo    6 May 2013 09:28:27 -0000
> @@ -21286,7 +21286,7 @@ is not set.
>  @item set history save
>  @itemx set history save on
>  Record command history in a file, whose name may be specified with  
> the
> -@code{set history filename} command.  By default, this option is  
> disabled.
> +@code{set history filename} command.  By default, this option is  
> enabled.
> 
>  @item set history save off
>  Stop recording command history in a file.
> Index: doc/ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
> retrieving revision 1.1452
> diff -u -p -r1.1452 ChangeLog
> --- doc/ChangeLog    29 Apr 2013 17:32:43 -0000    1.1452
> +++ doc/ChangeLog    6 May 2013 09:28:56 -0000
> @@ -1,3 +1,8 @@
> +2013-05-06  Muhammad Bilal  <mbilal@codesourcery.com>
> +
> +    PR gdb/15224:
> +    * gdb.texinfo (Command history): Set history save enabled.
This is not very clear. May be you can use something like "Mention that  
'set history save' is 'on' by default."

> +
>  2013-04-29  Tom Tromey  <tromey@redhat.com>
> 
>      PR python/14204:
> Index: NEWS
> ===================================================================
> RCS file: /cvs/src/src/gdb/NEWS,v
> retrieving revision 1.593
> diff -u -p -r1.593 NEWS
> --- NEWS    16 Apr 2013 14:36:53 -0000    1.593
> +++ NEWS    6 May 2013 09:30:13 -0000
> @@ -3,6 +3,9 @@
> 
>  *** Changes since GDB 7.6
> 
> +* The command 'history save' is now default enabled.
How about "is now enabled by default."

> +Disabled 'history save' on testsuiltes.
Maintainers can tell for sure but I think this does not need to be  
added in NEWS.

> +
>  * New commands:
>  catch rethrow
>    Like "catch throw", but catches a re-thrown exception.
> 
> 
> Thanks,
> -Bilal

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

[Pedro Alves]

On 05/07/2013 11:17 AM, Abid, Hafiz wrote:

>> +Disabled 'history save' on testsuiltes.
> Maintainers can tell for sure but I think this does not need to be added in NEWS.

Correct.

-- 
Pedro Alves

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

[mbilal]

On 05/06/2013 02:50 PM, mbilal wrote:
> Here is doc and news entry.
>
> Index: doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.1084
> diff -u -p -r1.1084 gdb.texinfo
> --- doc/gdb.texinfo    29 Apr 2013 17:32:43 -0000    1.1084
> +++ doc/gdb.texinfo    6 May 2013 09:28:27 -0000
> @@ -21286,7 +21286,7 @@ is not set.
>  @item set history save
>  @itemx set history save on
>  Record command history in a file, whose name may be specified with the
> -@code{set history filename} command.  By default, this option is 
> disabled.
> +@code{set history filename} command.  By default, this option is 
> enabled.
>
>  @item set history save off
>  Stop recording command history in a file.
> Index: doc/ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
> retrieving revision 1.1452
> diff -u -p -r1.1452 ChangeLog
> --- doc/ChangeLog    29 Apr 2013 17:32:43 -0000    1.1452
> +++ doc/ChangeLog    6 May 2013 09:28:56 -0000
> @@ -1,3 +1,8 @@
> +2013-05-06  Muhammad Bilal  <mbilal@codesourcery.com>
> +
> +    PR gdb/15224:
> +    * gdb.texinfo (Command history): Set history save enabled.
> +
>  2013-04-29  Tom Tromey  <tromey@redhat.com>
>
>      PR python/14204:
> Index: NEWS
> ===================================================================
> RCS file: /cvs/src/src/gdb/NEWS,v
> retrieving revision 1.593
> diff -u -p -r1.593 NEWS
> --- NEWS    16 Apr 2013 14:36:53 -0000    1.593
> +++ NEWS    6 May 2013 09:30:13 -0000
> @@ -3,6 +3,9 @@
>
>  *** Changes since GDB 7.6
>
> +* The command 'history save' is now default enabled.
> +Disabled 'history save' on testsuiltes.
> +
>  * New commands:
>  catch rethrow
>    Like "catch throw", but catches a re-thrown exception.
>
>
> Thanks,
> -Bilal
ping

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

[Eli Zaretskii]

> Date: Wed, 8 May 2013 13:24:41 +0500
> From: mbilal <mbilal@codesourcery.com>
> 
> On 05/06/2013 02:50 PM, mbilal wrote:
> > Here is doc and news entry.
> >
> > Index: doc/gdb.texinfo
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> > retrieving revision 1.1084
> > diff -u -p -r1.1084 gdb.texinfo
> > --- doc/gdb.texinfo    29 Apr 2013 17:32:43 -0000    1.1084
> > +++ doc/gdb.texinfo    6 May 2013 09:28:27 -0000
> > @@ -21286,7 +21286,7 @@ is not set.
> >  @item set history save
> >  @itemx set history save on
> >  Record command history in a file, whose name may be specified with the
> > -@code{set history filename} command.  By default, this option is 
> > disabled.
> > +@code{set history filename} command.  By default, this option is 
> > enabled.
> >
> >  @item set history save off
> >  Stop recording command history in a file.
> > Index: doc/ChangeLog
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
> > retrieving revision 1.1452
> > diff -u -p -r1.1452 ChangeLog
> > --- doc/ChangeLog    29 Apr 2013 17:32:43 -0000    1.1452
> > +++ doc/ChangeLog    6 May 2013 09:28:56 -0000
> > @@ -1,3 +1,8 @@
> > +2013-05-06  Muhammad Bilal  <mbilal@codesourcery.com>
> > +
> > +    PR gdb/15224:
> > +    * gdb.texinfo (Command history): Set history save enabled.
> > +
> >  2013-04-29  Tom Tromey  <tromey@redhat.com>
> >
> >      PR python/14204:
> > Index: NEWS
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/NEWS,v
> > retrieving revision 1.593
> > diff -u -p -r1.593 NEWS
> > --- NEWS    16 Apr 2013 14:36:53 -0000    1.593
> > +++ NEWS    6 May 2013 09:30:13 -0000
> > @@ -3,6 +3,9 @@
> >
> >  *** Changes since GDB 7.6
> >
> > +* The command 'history save' is now default enabled.
> > +Disabled 'history save' on testsuiltes.
> > +
> >  * New commands:
> >  catch rethrow
> >    Like "catch throw", but catches a re-thrown exception.
> >
> >
> > Thanks,
> > -Bilal
> ping

AFAIR, you got comments, but this patch does not reflect them.  Did
you send the correct diffs?

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

[Pedro Alves]

The default history file path also changed, and that needs
to be documented as well, both manual and NEWS.

On 05/08/2013 09:24 AM, mbilal wrote:
> On 05/06/2013 02:50 PM, mbilal wrote:
>> Here is doc and news entry.
>>
>> Index: doc/gdb.texinfo
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
>> retrieving revision 1.1084
>> diff -u -p -r1.1084 gdb.texinfo
>> --- doc/gdb.texinfo    29 Apr 2013 17:32:43 -0000    1.1084
>> +++ doc/gdb.texinfo    6 May 2013 09:28:27 -0000
>> @@ -21286,7 +21286,7 @@ is not set.
>>  @item set history save
>>  @itemx set history save on
>>  Record command history in a file, whose name may be specified with the
>> -@code{set history filename} command.  By default, this option is disabled.
>> +@code{set history filename} command.  By default, this option is enabled.
>>
>>  @item set history save off
>>  Stop recording command history in a file.
>> Index: doc/ChangeLog
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/doc/ChangeLog,v
>> retrieving revision 1.1452
>> diff -u -p -r1.1452 ChangeLog
>> --- doc/ChangeLog    29 Apr 2013 17:32:43 -0000    1.1452
>> +++ doc/ChangeLog    6 May 2013 09:28:56 -0000
>> @@ -1,3 +1,8 @@
>> +2013-05-06  Muhammad Bilal  <mbilal@codesourcery.com>
>> +
>> +    PR gdb/15224:
>> +    * gdb.texinfo (Command history): Set history save enabled.
>> +
>>  2013-04-29  Tom Tromey  <tromey@redhat.com>
>>
>>      PR python/14204:
>> Index: NEWS
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/NEWS,v
>> retrieving revision 1.593
>> diff -u -p -r1.593 NEWS
>> --- NEWS    16 Apr 2013 14:36:53 -0000    1.593
>> +++ NEWS    6 May 2013 09:30:13 -0000
>> @@ -3,6 +3,9 @@
>>
>>  *** Changes since GDB 7.6
>>
>> +* The command 'history save' is now default enabled.
>> +Disabled 'history save' on testsuiltes.
>> +
>>  * New commands:
>>  catch rethrow
>>    Like "catch throw", but catches a re-thrown exception.
>>
>>
>> Thanks,
>> -Bilal
> ping


-- 
Pedro Alves

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

[mbilal]

On 05/06/2013 02:45 PM, mbilal wrote:
> On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:
>
> > (4) Modify GDB testsuite to always disable history saving.
>
> patch for this.
>
> Index: lib/gdb.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
> retrieving revision 1.230
> diff -u -p -r1.230 gdb.exp
> --- lib/gdb.exp    15 Apr 2013 20:37:37 -0000    1.230
> +++ lib/gdb.exp    6 May 2013 07:01:55 -0000
> @@ -58,7 +58,7 @@ set BUILD_DATA_DIRECTORY "[pwd]/../data-
>  # INTERNAL_GDBFLAGS contains flags that the testsuite requires.
>  global INTERNAL_GDBFLAGS
>  if ![info exists INTERNAL_GDBFLAGS] {
> -    set INTERNAL_GDBFLAGS "-nw -nx -data-directory 
> $BUILD_DATA_DIRECTORY"
> +    set INTERNAL_GDBFLAGS "-nw -nx  -ex set\\ history\\ save\\ off 
> -data-directory $BUILD_DATA_DIRECTORY"
>  }
>
>  # The variable gdb_prompt is a regexp which matches the gdb prompt.
> Index: 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
> --- gdb.base/default.exp    15 Apr 2013 18:09:02 -0000    1.64
> +++ gdb.base/default.exp    6 May 2013 07:02:53 -0000
> @@ -521,8 +521,9 @@ gdb_test "set height" "Argument required
>  gdb_test_no_output "set history expansion" "set history expansion"
>  #test set history filename
>  gdb_test "set history filename" "Argument required .filename to set 
> it to.*" "set history filename"
> +#We are always disable history saving on testsuite
>  #test set history save
> -gdb_test_no_output "set history save" "set history save"
> +#gdb_test_no_output "set history save" "set history save"
>  #test set history size
>  gdb_test "set history size" "Argument required .integer to set it 
> to.*" "set history size"
>  #test set history
> @@ -644,7 +645,7 @@ gdb_test "show history expansion" "Histo
>  #test show history filename
>  gdb_test "show history filename" "The filename in which to record the 
> command history is.*.gdb_history.*" "show history filename"
>  #test show history save
> -gdb_test "show history save" "Saving of the history record on exit is 
> on." "show history save"
> +gdb_test "show history save" "Saving of the history record on exit is 
> off." "show history save"
>  #test show history size
>  gdb_test "show history size" "The size of the command history is.*" 
> "show history size"
>  #test show history
> Index: gdb.base/setshow.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/setshow.exp,v
> retrieving revision 1.30
> diff -u -p -r1.30 setshow.exp
> --- gdb.base/setshow.exp    24 Apr 2013 14:29:17 -0000    1.30
> +++ gdb.base/setshow.exp    6 May 2013 07:08:29 -0000
> @@ -174,10 +174,11 @@ gdb_test_no_output "set history filename
>      "set history filename foobar.baz"
>  #test show history filename foobar.baz
>  gdb_test "show history filename" "The filename in which to record the 
> command history is \"foobar.baz\"..*"  "show history filename 
> (foobar.baz)"
> +#We are always disable history saving on testsuite
>  #test set history save on
> -gdb_test_no_output "set history save on" "set history save on"
> +#gdb_test_no_output "set history save on" "set history save on"
>  #test show history save on
> -gdb_test "show history save" "Saving of the history record on exit is 
> on..*"  "show history save (on)"
> +gdb_test "show history save" "Saving of the history record on exit is 
> off..*"  "show history save (off)"
>  #test set history size 100
>  gdb_test_no_output "set history size 100" "set history size 100"
>  #test show history size 100
>
>
>
>
> Thanks,
> -Bilal
ping?

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

[mbilal]

On Wednesday, May 08, 2013 10:14 PM pedro Alves wrote:

 >> -    set INTERNAL_GDBFLAGS "-nw -nx -data-directory 
$BUILD_DATA_DIRECTORY"
 >> +    set INTERNAL_GDBFLAGS "-nw -nx  -ex set\\ history\\ save\\ off 
-data-directory $BUILD_DATA_DIRECTORY"

 >spurious space?               ------^^

Fixed.


 >> +#We are always disable history saving on testsuite
 >>  #test set history save
 >> -gdb_test_no_output "set history save" "set history save"
 >> +#gdb_test_no_output "set history save" "set history save"
 >>  #test set history size

 >I don't understand this.  Why disable the test?

We are disabling the history saving on all testsuit, if we run the above
testsuit then history saving becomes enable that will also writes the 
history
command on .gdb_history file. That's why I disabled this test case.



On Tuesday, May 07, 2013 2:40 PM Hafiz Abid wrote:

 >> +#We are always disable history saving on testsuite
 >s/are always disable/always disable/

fixed .



please find updated patch.

Index: testsuite/lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.230
diff -u -p -r1.230 gdb.exp
--- testsuite/lib/gdb.exp    15 Apr 2013 20:37:37 -0000    1.230
+++ testsuite/lib/gdb.exp    9 May 2013 10:38:18 -0000
@@ -58,7 +58,7 @@ set BUILD_DATA_DIRECTORY "[pwd]/../data-
  # INTERNAL_GDBFLAGS contains flags that the testsuite requires.
  global INTERNAL_GDBFLAGS
  if ![info exists INTERNAL_GDBFLAGS] {
-    set INTERNAL_GDBFLAGS "-nw -nx -data-directory $BUILD_DATA_DIRECTORY"
+    set INTERNAL_GDBFLAGS "-nw -nx -ex set\\ history\\ save\\ off 
-data-directory $BUILD_DATA_DIRECTORY"
  }
Index: 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
--- gdb.base/default.exp    15 Apr 2013 18:09:02 -0000    1.64
+++ gdb.base/default.exp    6 May 2013 07:02:53 -0000
@@ -521,8 +521,9 @@ gdb_test "set height" "Argument required
  gdb_test_no_output "set history expansion" "set history expansion"
  #test set history filename
  gdb_test "set history filename" "Argument required .filename to set it 
to.*" "set history filename"
+#We always disable history saving on testsuite
  #test set history save
-gdb_test_no_output "set history save" "set history save"
+#gdb_test_no_output "set history save" "set history save"
  #test set history size
  gdb_test "set history size" "Argument required .integer to set it 
to.*" "set history size"
  #test set history
@@ -644,7 +645,7 @@ gdb_test "show history expansion" "Histo
  #test show history filename
  gdb_test "show history filename" "The filename in which to record the 
command history is.*.gdb_history.*" "show history filename"
  #test show history save
-gdb_test "show history save" "Saving of the history record on exit is 
on." "show history save"
+gdb_test "show history save" "Saving of the history record on exit is 
off." "show history save"
  #test show history size
  gdb_test "show history size" "The size of the command history is.*" 
"show history size"
  #test show history
Index: gdb.base/setshow.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/setshow.exp,v
retrieving revision 1.30
diff -u -p -r1.30 setshow.exp
--- gdb.base/setshow.exp    24 Apr 2013 14:29:17 -0000    1.30
+++ gdb.base/setshow.exp    6 May 2013 07:08:29 -0000
@@ -174,10 +174,11 @@ gdb_test_no_output "set history filename
      "set history filename foobar.baz"
  #test show history filename foobar.baz
  gdb_test "show history filename" "The filename in which to record the 
command history is \"foobar.baz\"..*"  "show history filename (foobar.baz)"
+#We always disable history saving on testsuite
  #test set history save on
-gdb_test_no_output "set history save on" "set history save on"
+#gdb_test_no_output "set history save on" "set history save on"
  #test show history save on
-gdb_test "show history save" "Saving of the history record on exit is 
on..*"  "show history save (on)"
+gdb_test "show history save" "Saving of the history record on exit is 
off..*"  "show history save (off)"
  #test set history size 100
  gdb_test_no_output "set history size 100" "set history size 100"
  #test show history size 100
Index: testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.3640
diff -u -p -r1.3640 ChangeLog
--- testsuite/ChangeLog    30 Apr 2013 12:33:51 -0000    1.3640
+++ testsuite/ChangeLog    9 May 2013 11:04:17 -0000
@@ -1,3 +1,11 @@
+2013-05-06  Muhammad Bilal  <mbilal@codesourcery.com>
+
+    PR gdb/15224
+    * lib/gdb.exp: Disabled history saving on testsuites.
+    * gdb.base/default.exp: Comment 'set history save' test case.
+    Changed out put of 'show history save' test case.
+    * gdb.base/setshow.exp: Likewise.
+
  2013-03-27  Walfred Tedeschi  <walfred.tedeschi@intel.com>

      * gdb.xml/maint_print_struct.exp: New file.


Thanks,
-Bilal

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

[Pedro Alves]

On 05/09/2013 12:12 PM, mbilal wrote:
> On Wednesday, May 08, 2013 10:14 PM pedro Alves wrote:
> 
>>> -    set INTERNAL_GDBFLAGS "-nw -nx -data-directory $BUILD_DATA_DIRECTORY"
>>> +    set INTERNAL_GDBFLAGS "-nw -nx  -ex set\\ history\\ save\\ off -data-directory $BUILD_DATA_DIRECTORY"
> 
>>spurious space?               ------^^
> 
> Fixed.

BTW, I think:

    set INTERNAL_GDBFLAGS "-nw -nx -ex \"set history save off\" -data-directory $BUILD_DATA_DIRECTORY"

would be easier to read (and write).

>>> +#We are always disable history saving on testsuite
>>>  #test set history save
>>> -gdb_test_no_output "set history save" "set history save"
>>> +#gdb_test_no_output "set history save" "set history save"
>>>  #test set history size
> 
>>I don't understand this.  Why disable the test?
> 
> We are disabling the history saving on all testsuit, if we run the above
> testsuit then history saving becomes enable that will also writes the history
> command on .gdb_history file. That's why I disabled this test case.

So that's a preexisting bug, that triggers even if GDB's
default isn't changed, right?  Let's fix it first, before the default is
changed, in its own patch.  Please send it as a new thread (i.e., not in
reply to this thread, in a message that includes a self-contained
description of the rationale for the change, and a concise Subject:.

I suggest perusing these guidelines that although written for the
Linux kernel, can be transcribed to lots of other projects as well:
<http://www.linux-mips.org/wiki/Submitting_Patches>

Instead of disabling the test completely, I suggest re-disabling
history saving immediately afterwards:

 gdb_test_no_output "set history save" "set history save"
+gdb_test_no_output "set history save off" "set history save off"

-- 
Pedro Alves

Re: [PATCH 4/7] Disable history saving for testsuit

[mbilal]

[-- Attachment #1: Type: text/plain, Size: 257 bytes --]

HI,
Please find attached updated patch.
In this patch, History saving is being disabled for all testsuite.


2013-05-06  Muhammad Bilal <mbilal@codesourcery.com>

     PR gdb/15224
     * lib/gdb.exp: Disable history saving for testsuites.


Thanks,
-Bilal

[-- Attachment #2: 4 --]
[-- Type: text/plain, Size: 609 bytes --]

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index b8a7cf8..a1a131e 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -58,7 +58,7 @@ set BUILD_DATA_DIRECTORY "[pwd]/../data-directory"
 # INTERNAL_GDBFLAGS contains flags that the testsuite requires.
 global INTERNAL_GDBFLAGS
 if ![info exists INTERNAL_GDBFLAGS] {
-    set INTERNAL_GDBFLAGS "-nw -nx -data-directory $BUILD_DATA_DIRECTORY"
+    set INTERNAL_GDBFLAGS "-nw -nx -ex \"set history save off\" -data-directory $BUILD_DATA_DIREC
 }
 
 # The variable gdb_prompt is a regexp which matches the gdb prompt.

Re: [PATCH 4/7] Disable history saving for testsuit

[mbilal]

On 05/15/2013 03:56 PM, mbilal wrote:
> HI,
> Please find attached updated patch.
> In this patch, History saving is being disabled for all testsuite.
>
>
> 2013-05-06  Muhammad Bilal <mbilal@codesourcery.com>
>
>     PR gdb/15224
>     * lib/gdb.exp: Disable history saving for testsuites.
>
>
> Thanks,
> -Bilal
ping?

Re: [PATCH 4/7] Disable history saving for testsuit

[Pedro Alves]

On 05/15/2013 11:56 AM, mbilal wrote:
> HI,
> Please find attached updated patch.
> In this patch, History saving is being disabled for all testsuite.
> 
> 
> 2013-05-06  Muhammad Bilal <mbilal@codesourcery.com>

Double space after name.

> 
>     PR gdb/15224
>     * lib/gdb.exp: Disable history saving for testsuites.

Say:

	* lib/gdb.exp: Add '-ex "set history save off"' to
	INTERNAL_GDBFLAGS

Okay with those changes.

-- 
Pedro Alves

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

[Pedro Alves]

On 05/06/2013 10:43 AM, mbilal wrote:
> On Wednesday, April 03, 2013 7:30 PM Jan Kratochvil wrote:
> 
>> (3) Fix currently incorrectly touched history file even if no commands get
> 
> here is patch

Missing ChangeLog entry.

> 
> 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 06:16:04 -0000
> @@ -1430,7 +1430,8 @@ quit_force (char *args, int from_tty)
>    /* Save the history information if it is appropriate to do so. */
>    DO_TRY
>      {
> -      if (write_history_p && history_filename)
> +      if (write_history_p && history_filename
> +          && input_from_terminal_p())

Missing space before parens.

>      write_history (history_filename);
>      }
>    DO_PRINT_EX;
> 
> 
> 
> I have tested

It'd be great if we had that in our testsuite.

-- 
Pedro Alves

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

[Pedro Alves]

Please do this in the command's set handler rather than
hacking in the the set/show machinery.

Please give each patch a distinct subject next time.

(Including a self-contained description of what's wrong,
and what and how the patch fixes it in the submission
is also useful for both review, and future archaeology.)

Thanks,
-- 
Pedro Alves

Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path

[mbilal]

Here is a updated patch for Fix relative "set history filename" to by 
immediately converted to absolute
On Monday, May 06, 2013 4:26 PM Pedro Alves wrote:

>Please do this in  the command's set handler rather than
 >hacking in the the set/show machinery.

Yes,
Here I am printing the absolute path in show_history_filename ,
I think it is more appropriate than using the internal set/show machinery .

diff --git a/gdb/top.c b/gdb/top.c
index 480b67e..ae93e21 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -729,6 +729,9 @@ static void
  show_history_filename (struct ui_file *file, int from_tty,
                        struct cmd_list_element *c, const char *value)
  {
+  if (!IS_ABSOLUTE_PATH (value))
+    value = concat (current_directory, "/", value,
+                   (char *)NULL);
    fprintf_filtered (file, _("The filename in which to record "
                             "the command history is \"%s\".\n"),
                     value);
Index: gdb.base/setshow.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/setshow.exp,v
retrieving revision 1.30
diff -u -p -r1.30 setshow.exp
--- gdb.base/setshow.exp    24 Apr 2013 14:29:17 -0000    1.30
+++ gdb.base/setshow.exp    6 May 2013 06:01:05 -0000
@@ -173,7 +173,8 @@ gdb_test "show history expansion on" "Hi
  gdb_test_no_output "set history filename foobar.baz" \
      "set history filename foobar.baz"
  #test show history filename foobar.baz
-gdb_test "show history filename" "The filename in which to record the 
command history is \"foobar.baz\"..*"  "show history filename (foobar.baz)"
+gdb_test "show history filename" "The filename in which to record the 
command history is \"[file join [pwd] foobar.baz]\"..*" \
+    "show history filename \"[file join [pwd] foobar.baz]\"."
  #test set history save on
  gdb_test_no_output "set history save on" "set history save on"
  #test show history save on


Thanks,
-Bilal

Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path

[Pedro Alves]

On 05/07/2013 08:19 AM, mbilal wrote:
> Here is a updated patch for Fix relative "set history filename" to by immediately converted to absolute
> On Monday, May 06, 2013 4:26 PM Pedro Alves wrote:
> 
>> Please do this in  the command's set handler rather than
>>hacking in the the set/show machinery.
> 
> Yes,
> Here I am printing the absolute path in show_history_filename ,

No, sorry, this doesn't fix the issue I originally pointed out
needed fixing.

-- 
Pedro Alves

Re: [PATCH 2/7] PR gdb/15224 fix for Unify interactivity tests to use input_from_terminal_p

[mbilal]

Here is a updated patch for fix for Unify interactivity tests to use 
input_from_terminal_p:
On Monday, May 06, 2013 4:26 PM Pedro Alves wrote:

>Please do this in  the command's set handler rather than
 >hacking in the the set/show machinery.

Thanks Pedro for correct guide line

Index: event-top.c
===================================================================
RCS file: /cvs/src/src/gdb/event-top.c,v
retrieving revision 1.94
diff -u -p -r1.94 event-top.c
--- event-top.c    21 Mar 2013 17:37:29 -0000    1.94
+++ event-top.c    7 May 2013 07:03:33 -0000
@@ -604,7 +604,8 @@ command_line_handler (char *rl)

    /* Add line to history if appropriate.  */
    if (instream == stdin
-      && ISATTY (stdin) && *linebuffer)
+      && ISATTY (stdin) && *linebuffer
+      && input_from_terminal_p())
      add_history (linebuffer);

    /* Note: lines consisting solely of comments are added to the command




And please help me to review other patchesas well.


Thanks,
-Bilal

Re: [PATCH 2/7] PR gdb/15224 fix for Unify interactivity tests to use input_from_terminal_p

[mbilal]

On 05/07/2013 12:26 PM, mbilal wrote:
> Here is a updated patch for fix for Unify interactivity tests to use 
> input_from_terminal_p:
> On Monday, May 06, 2013 4:26 PM Pedro Alves wrote:
>
>> Please do this in  the command's set handler rather than
> >hacking in the the set/show machinery.
>
> Thanks Pedro for correct guide line
>
> Index: event-top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/event-top.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 event-top.c
> --- event-top.c    21 Mar 2013 17:37:29 -0000    1.94
> +++ event-top.c    7 May 2013 07:03:33 -0000
> @@ -604,7 +604,8 @@ command_line_handler (char *rl)
>
>    /* Add line to history if appropriate.  */
>    if (instream == stdin
> -      && ISATTY (stdin) && *linebuffer)
> +      && ISATTY (stdin) && *linebuffer
> +      && input_from_terminal_p())
>      add_history (linebuffer);
>
>    /* Note: lines consisting solely of comments are added to the command
>
>
>
>
> And please help me to review other patchesas well.
>
>
> Thanks,
> -Bilal
ping?

Re: [PATCH 2/7] PR gdb/15224 fix for Unify interactivity tests to use input_from_terminal_p

[Pedro Alves]

Hi,

Thanks for the patch.

> ping?

Please give it at least a week before pinging.

On 05/08/2013 09:22 AM, mbilal wrote:
> On 05/07/2013 12:26 PM, mbilal wrote:
>> Here is a updated patch for fix for Unify interactivity tests to use input_from_terminal_p:

>> Index: event-top.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/event-top.c,v
>> retrieving revision 1.94
>> diff -u -p -r1.94 event-top.c
>> --- event-top.c    21 Mar 2013 17:37:29 -0000    1.94
>> +++ event-top.c    7 May 2013 07:03:33 -0000
>> @@ -604,7 +604,8 @@ command_line_handler (char *rl)
>>
>>    /* Add line to history if appropriate.  */
>>    if (instream == stdin
>> -      && ISATTY (stdin) && *linebuffer)
>> +      && ISATTY (stdin) && *linebuffer
>> +      && input_from_terminal_p())
>>      add_history (linebuffer);

I'd guess that input_from_terminal_p already does some of the
checks this is also making, so that this could be simplified.
Missing space before parens.

A ChangeLog entry is missing.

-- 
Pedro Alves

Re: [PATCH 2/7] PR gdb/15224 fix for Unify interactivity tests to use input_from_terminal_p

[mbilal]

On Wednesday, May 08, 2013 9:22 PM Pedro Alves wrote:

 >I'd guess that input_from_terminal_p already does some of the
 >checks this is also making, so that this could be simplified.

I don't think that because input_from_terminal_p
checks the condition instream==stdin that
is equal in both sanario , but input_from_terminal_p
also return 1 when instream==NULL.
So I think that there is no any condition that is overlapping with
input_from_terminal_p.


 >Missing space before parens.

fixed.

 >A ChangeLog entry is missing.

fixed.

Please find updated patch.

Index: event-top.c
===================================================================
RCS file: /cvs/src/src/gdb/event-top.c,v
retrieving revision 1.94
diff -u -p -r1.94 event-top.c
--- event-top.c    21 Mar 2013 17:37:29 -0000    1.94
+++ event-top.c    9 May 2013 10:23:21 -0000
@@ -604,7 +604,8 @@ command_line_handler (char *rl)

    /* Add line to history if appropriate.  */
    if (instream == stdin
-      && ISATTY (stdin) && *linebuffer)
+      && ISATTY (stdin) && *linebuffer
+      && input_from_terminal_p ())
      add_history (linebuffer);
Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.15497
diff -u -p -r1.15497 ChangeLog
--- ChangeLog    1 May 2013 14:54:18 -0000    1.15497
+++ ChangeLog    9 May 2013 10:28:41 -0000
@@ -1,3 +1,9 @@
+2013-05-09  Muhammad Bilal  <mbilal@codesourcery.com>
+
+    PR gdb/15224
+    * event-top.c (command_line_handler): Use input_from_terminal_p 
condition
+    for add line to history.
+
  2013-05-01  Joel Brobecker  <brobecker@adacore.com>

      * darwin-nat.c (darwin_read_write_inferior): Change types

Re: [PATCH 2/7] PR gdb/15224 fix for Unify interactivity tests to use input_from_terminal_p

[Pedro Alves]

On 05/09/2013 12:08 PM, mbilal wrote:
> On Wednesday, May 08, 2013 9:22 PM Pedro Alves wrote:
> 
>>I'd guess that input_from_terminal_p already does some of the
>>checks this is also making, so that this could be simplified.
> 
> I don't think that because input_from_terminal_p
> checks the condition instream==stdin that
> is equal in both sanario , but input_from_terminal_p
> also return 1 when instream==NULL.
> So I think that there is no any condition that is overlapping with
> input_from_terminal_p.

  /* If INSTREAM is unset, and we are not in a user command, we
     must be in Insight.  That's like having a terminal, for our
     purposes.  */
  if (instream == NULL && !in_user_command)
    return 1;

This very much looks like a case where history should work, but
won't unless we simplify to check input_from_terminal_p only.

The point of unifying interactivity tests is exactly so that
all these little details are centralized in a single place.

Doesn't

-    if (instream == stdin
-      && ISATTY (stdin) && *linebuffer)
+    if (*linebuffer && input_from_terminal_p ())

work?  If not, why not?  It'd be great to try that on
GNU/Linux, mingw32 gdb running under cmd.exe, and mingw32 gdb under
Cygwin bash, the latter with and without "set interactive-mode on".
I suspect this change would fix history for the forced
"set interactive-mode on" case too.

> Index: event-top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/event-top.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 event-top.c
> --- event-top.c    21 Mar 2013 17:37:29 -0000    1.94
> +++ event-top.c    9 May 2013 10:23:21 -0000
> @@ -604,7 +604,8 @@ command_line_handler (char *rl)
> 
>    /* Add line to history if appropriate.  */
>    if (instream == stdin
> -      && ISATTY (stdin) && *linebuffer)
> +      && ISATTY (stdin) && *linebuffer
> +      && input_from_terminal_p ())
>      add_history (linebuffer);

Please grep for add_history.  There's at least one more call in
the tree.

-- 
Pedro Alves

Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path

[mbilal]

please find updated patch .

I made new 'set_history_filename' function to solve this problem

and thanks Pedro for review this patch.



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);
+}
+
  /* Init the history buffer.  Note that we are called after the init 
file(s)
     have been read so that the user can change the history file via his
     .gdbinit file (for instance).  The GDBHISTFILE environment variable
@@ -1777,7 +1786,7 @@ variable \"HISTSIZE\", or to 256 if this variable 
is not set."),
  Set the filename in which to record the command history"), _("\
  Show the filename in which to record the command history"), _("\
  (the list of previous commands of which a record is kept)."),
-                           NULL,
+                           set_history_filename,
                             show_history_filename,
                             &sethistlist, &showhistlist);




Thanks,
-Bilal

Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path

[mbilal]

On 05/07/2013 05:43 PM, mbilal wrote:
> please find updated patch .
>
> I made new 'set_history_filename' function to solve this problem
>
> and thanks Pedro for review this patch.
>
>
>
> 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);
> +}
> +
>  /* Init the history buffer.  Note that we are called after the init 
> file(s)
>     have been read so that the user can change the history file via his
>     .gdbinit file (for instance).  The GDBHISTFILE environment variable
> @@ -1777,7 +1786,7 @@ variable \"HISTSIZE\", or to 256 if this 
> variable is not set."),
>  Set the filename in which to record the command history"), _("\
>  Show the filename in which to record the command history"), _("\
>  (the list of previous commands of which a record is kept)."),
> -                           NULL,
> +                           set_history_filename,
>                             show_history_filename,
>                             &sethistlist, &showhistlist);
>
>
>
>
> Thanks,
> -Bilal
ping?

Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path

[mbilal]

[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]

Hi,
Please find attached updated patch.

On Wednesday, May 08, 2013 8:47 PM Pedro wrote:

>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:

Fixed

On Wednesday, May 08, 2013 10:25 PM Pedro wrote:
>In addition to my previous comments,  I realized that this will do the
 >wrong thing with "set history filename ~/foo".  Best use tilde_expand
 >and gdb_realpath

'set history filename ~/foo' is working because following code is doing
same as you described . I have also attached test case for this.

if (arg != NULL)
       {
         /* Clear trailing whitespace of filename.  */
         char *ptr = arg + strlen (arg) - 1;

         while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
           ptr--;
         *(ptr + 1) = '\0';

         val = tilde_expand (arg);
       }
     else
       val = xstrdup ("");

     if (*(char **) c->var == NULL
         || strcmp (*(char **) c->var, val) != 0)
       {
         xfree (*(char **) c->var);
         *(char **) c->var = val;

         option_changed = 1;




2013-05-15  Muhammad Bilal <mbilal@codesourcery.com>

     PR gdb/15224
     * top.c (set_history_filename): New function.



2013-05-15  Muhammad Bilal <mbilal@codesourcery.com>

     PR gdb/15224
     * gdb.base/setshow.exp: Test 'set history filename' relative
     path.





Thanks,
-Bilal

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 3041 bytes --]

diff --git a/gdb/top.c b/gdb/top.c
index 480b67e..45b83d7 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"
@@ -1693,6 +1694,17 @@ show_exec_done_display_p (struct ui_file *file, int from_tty,
                    value);
 }
 
+static void
+set_history_filename (char *args, int from_tty, struct cmd_list_element *c)
+{
+  /* 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.  */
+  if (!IS_ABSOLUTE_PATH (history_filename))
+    history_filename = concat (current_directory, "/", history_filename,
+                              (char *) NULL);
+}
+
 /* "set" command for the gdb_datadir configuration variable.  */
 
 static void
@@ -1777,7 +1789,7 @@ variable \"HISTSIZE\", or to 256 if this variable is not set."),
 Set the filename in which to record the command history"), _("\
 Show the filename in which to record the command history"), _("\
 (the list of previous commands of which a record is kept)."),
-                           NULL,
+                           set_history_filename,
                            show_history_filename,
                            &sethistlist, &showhistlist);
diff --git a/gdb/testsuite/gdb.base/setshow.exp b/gdb/testsuite/gdb.base/setshow.exp
index 6d250c0..5345b4b 100644
--- a/gdb/testsuite/gdb.base/setshow.exp
+++ b/gdb/testsuite/gdb.base/setshow.exp
@@ -169,11 +169,24 @@ gdb_test_no_output "set height unlimited"
 gdb_test_no_output "set history expansion on" "set history expansion on" 
 #test show history expansion on
 gdb_test "show history expansion on" "History expansion on command input is on.*" "show history expansion" 
+#get home directory path
+gdb_test_multiple "show environment HOME" "show home directory" {
+    -re "\nHOME = (.*).\n.*$gdb_prompt $" {
+       set HOME $expect_out(1,string)    
+    }
+}
+#test set history filename ~/foobar.baz
+gdb_test_no_output "set history filename ~/foobar.baz" \
+    "set history filename ~/foobar.baz" 
+#test show history filename ~/foobar.baz
+gdb_test "show history filename" "The filename in which to record the command history is \"[file join $HOME foobar.baz]\"..*" \
+    "show history filename ([file join $HOME foobar.baz])"     
 #test set history filename foobar.baz
 gdb_test_no_output "set history filename foobar.baz" \
     "set history filename foobar.baz" 
 #test show history filename foobar.baz
-gdb_test "show history filename" "The filename in which to record the command history is \"foobar.baz\"..*"  "show history filename (foobar.baz)" 
+gdb_test "show history filename" "The filename in which to record the command history is \"[file join [pwd] foobar.baz]\"..*" \
+    "show history filename ([file join [pwd] foobar.baz])" 
 #test set history save on
 gdb_test_no_output "set history save on" "set history save on" 
 #test show history save on

Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path

[Pedro Alves]

On 05/15/2013 11:46 AM, mbilal wrote:

> On Wednesday, May 08, 2013 10:25 PM Pedro wrote:
>> In addition to my previous comments,  I realized that this will do the
>>wrong thing with "set history filename ~/foo".  Best use tilde_expand
>>and gdb_realpath
> 
> 'set history filename ~/foo' is working because following code is doing
> same as you described . I have also attached test case for this.

Ah, good.

> 2013-05-15  Muhammad Bilal <mbilal@codesourcery.com>
> 
>     PR gdb/15224

It would have been better to file a different PR for this
one, and then making 15224 depend on it.  This is a preexisting
bug, after all, that can be triggered no matter what the
default history filename is.

>     * top.c (set_history_filename): New function.

This is incomplete.  You must have done something with
the new function.  :-)

> 2013-05-15  Muhammad Bilal <mbilal@codesourcery.com>
> 
>     PR gdb/15224
>     * gdb.base/setshow.exp: Test 'set history filename' relative
>     path.

I think you meant "with a relative path".  But this tests more now.
Please make it complete.


> diff --git a/gdb/top.c b/gdb/top.c
> index 480b67e..45b83d7 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"
> @@ -1693,6 +1694,17 @@ show_exec_done_display_p (struct ui_file *file, int from_tty,
>                     value);
>  }
>  
> +static void
> +set_history_filename (char *args, int from_tty, struct cmd_list_element *c)
> +{
> +  /* 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.  */
> +  if (!IS_ABSOLUTE_PATH (history_filename))
> +    history_filename = concat (current_directory, "/", history_filename,
> +                              (char *) NULL);
> +}

This leaks the previous history_filename.  You can use reconcat
instead to address that.

> +
>  /* "set" command for the gdb_datadir configuration variable.  */
>  
>  static void
> @@ -1777,7 +1789,7 @@ variable \"HISTSIZE\", or to 256 if this variable is not set."),
>  Set the filename in which to record the command history"), _("\
>  Show the filename in which to record the command history"), _("\
>  (the list of previous commands of which a record is kept)."),
> -                           NULL,
> +                           set_history_filename,
>                             show_history_filename,
>                             &sethistlist, &showhistlist);
> diff --git a/gdb/testsuite/gdb.base/setshow.exp b/gdb/testsuite/gdb.base/setshow.exp
> index 6d250c0..5345b4b 100644
> --- a/gdb/testsuite/gdb.base/setshow.exp
> +++ b/gdb/testsuite/gdb.base/setshow.exp
> @@ -169,11 +169,24 @@ gdb_test_no_output "set height unlimited"
>  gdb_test_no_output "set history expansion on" "set history expansion on" 
>  #test show history expansion on
>  gdb_test "show history expansion on" "History expansion on command input is on.*" "show history expansion" 
> +#get home directory path
> +gdb_test_multiple "show environment HOME" "show home directory" {
> +    -re "\nHOME = (.*).\n.*$gdb_prompt $" {
> +       set HOME $expect_out(1,string)    
> +    }
> +}
> +#test set history filename ~/foobar.baz
> +gdb_test_no_output "set history filename ~/foobar.baz" \
> +    "set history filename ~/foobar.baz" 
> +#test show history filename ~/foobar.baz
> +gdb_test "show history filename" "The filename in which to record the command history is \"[file join $HOME foobar.baz]\"..*" \
> +    "show history filename ([file join $HOME foobar.baz])"     

The gdb_test_multiple above only sets HOME on success.  That means this will
explode with an access to an unknown symbol ($HOME) if the gdb_test_multiple
fails.

You can put the result of [file join $HOME foobar.baz] in a
variable and use that, instead of doing that twice:

  set filename [file join $HOME foobar.baz]

Actually, "file join" uses the build system's directory separator, but we
want the host's.  Actually^2, GDB always uses '/' here, so we can ignore
that and just do:

  set filename "$HOME/foobar.baz"

Also,

+gdb_test "show history filename" "The filename in which to record the command history is \"[file join $HOME foobar.baz]\"..*" \
+    "show history filename ([file join $HOME foobar.baz])"

Don't put that "([file join $HOME foobar.baz])" in the gdb.sum message,
as that will make the message depend on system/whoever runs it.

>  #test set history filename foobar.baz
>  gdb_test_no_output "set history filename foobar.baz" \
>      "set history filename foobar.baz" 
>  #test show history filename foobar.baz
> -gdb_test "show history filename" "The filename in which to record the command history is \"foobar.baz\"..*"  "show history filename (foobar.baz)" 
> +gdb_test "show history filename" "The filename in which to record the command history is \"[file join [pwd] foobar.baz]\"..*" \
> +    "show history filename ([file join [pwd] foobar.baz])" 

Another instance of the same problem mentioned before.  [pwd]
returns the current working directory on the build machine,
not the host's.  Use gdb's "pwd", not tcl's.

As long as we're breaking the long line with a \, please do it
before "The filename" too, so the line ends up a little shorter.

>  #test set history save on
>  gdb_test_no_output "set history save on" "set history save on" 
>  #test show history save on
> 

-- 
Pedro Alves

Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path

[mbilal]

On 05/15/2013 03:43 PM, mbilal wrote:
> Hi,
> Please find attached updated patch.
>
> On Wednesday, May 08, 2013 8:47 PM Pedro wrote:
>
> >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:
>
> Fixed
>
> On Wednesday, May 08, 2013 10:25 PM Pedro wrote:
> >In addition to my previous comments, I realized that this will do the
> >wrong thing with "set history filename ~/foo".  Best use tilde_expand
> >and gdb_realpath
>
> 'set history filename ~/foo' is working because following code is doing
> same as you described . I have also attached test case for this.
>
> if (arg != NULL)
>       {
>         /* Clear trailing whitespace of filename.  */
>         char *ptr = arg + strlen (arg) - 1;
>
>         while (ptr >= arg && (*ptr == ' ' || *ptr == '\t'))
>           ptr--;
>         *(ptr + 1) = '\0';
>
>         val = tilde_expand (arg);
>       }
>     else
>       val = xstrdup ("");
>
>     if (*(char **) c->var == NULL
>         || strcmp (*(char **) c->var, val) != 0)
>       {
>         xfree (*(char **) c->var);
>         *(char **) c->var = val;
>
>         option_changed = 1;
>
>
>
>
> 2013-05-15  Muhammad Bilal <mbilal@codesourcery.com>
>
>     PR gdb/15224
>     * top.c (set_history_filename): New function.
>
>
>
> 2013-05-15  Muhammad Bilal <mbilal@codesourcery.com>
>
>     PR gdb/15224
>     * gdb.base/setshow.exp: Test 'set history filename' relative
>     path.
>
>
>
>
>
> Thanks,
> -Bilal
>
>
>
ping?

Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path

[Pedro Alves]

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

Re: [PATCH 1/7] PR gdb/15224 "set history filename" to by immediately converted to absolute path

[Pedro Alves]

On 05/08/2013 04:46 PM, Pedro Alves wrote:

>> +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);
>> +}

In addition to my previous comments, I realized that this will do the
wrong thing with "set history filename ~/foo".  Best use tilde_expand
and gdb_realpath.

-- 
Pedro Alves

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

[Abid, Hafiz]

It will be easier if you include the relevent ChangeLogs for each patch  
in the corresponding email.  It makes it easy to review and make sure  
that ChangeLog covers the change.  I also noticed that you have not  
included any ChangeLog entries for the testsuite.

Regards,
Abid

On 06/05/13 10:38:58, mbilal wrote:
> Hi,
> 
> Here is the proposed patch series of 'set history save on' by default
> A thread has been discussed here  
> http://sourceware.org/ml/gdb-patches/2013-04/msg00067.html
> 
> 
> 
> 
> Index: ChangeLog
> ===================================================================
> RCS file: /cvs/src/src/gdb/ChangeLog,v
> retrieving revision 1.15497
> diff -u -p -r1.15497 ChangeLog
> --- ChangeLog    1 May 2013 14:54:18 -0000    1.15497
> +++ ChangeLog    6 May 2013 09:33:22 -0000
> @@ -1,3 +1,16 @@
> +2013-05-06  Muhammad Bilal  <mbilal@codesourcery.com>
> +
> +    PR gdb/15224
> +    * top.c (init_main): Modified write_history_p.
> +    (quit_force): Add condition input_from_terminal_p.
> +    (command_line_input): Unify interactivity tests to
> +    use input_from_terminal_p.
> +
> +    * cli/cli-setshow.c (do_set_command): set history filaname
> +    with absolute patch.
> +
> +    * NEWS: Updated.
> +
> 
> 
> Thanks,
> -Bilal

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

[Jan Kratochvil]

On Mon, 06 May 2013 11:38:58 +0200, mbilal wrote:
> Here is the proposed patch series of 'set history save on' by default
> A thread has been discussed here
> http://sourceware.org/ml/gdb-patches/2013-04/msg00067.html

This was leading to getting accepted but there were too many threaded mails,
it would be great if you could resubmit it as completely new mails (no
replies), best a new mail for every patch part.  Additionally a GIT branch for
it would be even better but that is not required.


Thanks,
Jan