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

[Pedro Alves <palves@redhat.com>]

On 04/01/2013 01:29 PM, Jan Kratochvil wrote:
>>> > > I was thinking to just bind the default WRITE_HISTORY_P to
>>> > > whether ~/.gdbinit is being read or not.
>> > 
>> > The history's filename is also tweakable with the GDBHISTFILE
>> > environment variable (there's HISTSIZE too; 'GDBHISTFILE= gdb' effectively
>> > disables history load/save).  Doesn't feel quite right to tie this to .gdbinit.
> It matters how are the existing environments set up.  And by default neither
> HISTSIZE nor GDBHISTFILE environment variables are set.

Not clear to me which environments you're referring to here, if you're
referring to any specific environments.  (HISTSIZE won't be as rare,
because that's also the variable used by bash.  We should probably
have a GDBHISTSIZE too.)

> Binding it to ~/.gdbinit is really ugly but I did not have a better idea
> compatible with existing GDB scripts/frontends/testsuites.

MI frontends are not an issue currently, as discussed.  We could add
a check for annotations too, if necessary.

We don't add commands to history unless we're interactive debugging:

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

and init_history (what loads history) isn't reached with either

 $ gdb -ex "quit"

or

 $ echo "quit" > cmd
 $ gdb -x ./cmd

Also,

 $ gdb --batch

never reaches either add_history or init_history.

I think however, that whether to write history to file should also
check whether we're in interactive debugging mode:

  /* Save the history information if it is appropriate to do so.  */
  if (write_history_p && history_filename)
    write_history (history_filename);

That is, e.g., this touches history:

 $ ls -als ~/.gdbhist
 4 -rw-------. 1 pedro pedro 27 Apr  1 14:10 /home/pedro/.gdbhist
 $ date
 Mon Apr  1 14:37:01 WEST 2013
 $ gdb -ex "set history filename ~/.gdbhist" < /dev/null
 (gdb) quit
 $ ls -als ~/.gdbhist
 4 -rw-------. 1 pedro pedro 27 Apr  1 14:37 /home/pedro/.gdbhist

That, I'd call a bug.

Probably, it'd be best to use input_from_terminal_p, which also
checks for --batch mode, among other things.

I think that all covers cases like abrt and other crash catchers that
spawn gdb?

As for testsuites and similar setups, I'm thinking the cases that could
be affected would be restricted to setups like ours and gcc's, that drive
gdb with expect, which is precisely designed to launch programs
interactively, as if a user was running them, so it'd make a lot of sense
to me to not treat them specially (having them adjust instead).

>> > One possible counter argument to flipping this on would that since history
>> > files are saved to the current directory by default, this has potential for
>> > littering users' directories with (hidden) .gdb_history files without the
>> > users noticing.  We should at least make that fact explicit in NEWS and
>> > perhaps make it prominent in the manual.
> 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.

That'd be fine with me.  However, we'd need a way to be able to specify
"./.gdbinit" back.  We have precedent for "$cwd", so perhaps that would
best be addressed by making "set history filename $cwd/.gdb_history" work.

-- 
Pedro Alves