Re: [PATCH] Don't truncate the history file when history size is unlimited

[Patrick Palka <patrick@parcs.ath.cx>]

On Mon, Jun 15, 2015 at 11:25 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/09/2015 08:27 PM, Patrick Palka wrote:
>> We still do not handle "set history size unlimited" correctly.  In
>> particular, after writing to the history file, we truncate the history
>> even if it is unlimited.
>
> Whoops.
>
>>
>> This patch makes sure that we do not call history_truncate_file() if the
>> history is not stifled (i.e. if it's unlimited).  This bug causes the
>> history file to be truncated to zero on exit when one has "set history
>> size unlimited" in their gdbinit file.  Although this code exists in GDB
>> 7.8 it is masked by a pre-existing bug that's been only fixed in GDB 7.9
>> (PR gdb/17820).
>>
>> I tried to make a test to check that the history does not get truncated
>> on exit when the history size is unlimited, but I could not get the test
>> to work properly.
>
> How so?

I could not observe that the history file gets truncated with this
patch reverted.  Maybe I simply have forgotten to revert the patch or
something stupid like that.  I'll take another stab at it.

>
>> Also I could not figure out a good way to create a
>> temporary file (to act as the history file) in tcl versions earlier than
>> 8.6.  I am not sure if it's worth the effort to add a test.
>
> Not sure I follow.  Why do you need a temporary file?
> You can leave the file in the build/test dir.  In fact, that's
> encouraged, to make it easier to debug the test on failures.
> You can instead delete a stale file from a previous run at the
> start of the (new) run.

Good point... What's the right way to refer to the
$buildroot/gdb/testsuite/gdb.base directory?

>
>> (Apparently I did not test my changes to history file handling
>> extensively enough.  Sorry..)
>
> Don't stress about it.  But, it does show out that having a test
> would be useful.  So I think it's worth the effort.
>
> The code changes look OK to me.
>
> Thanks,
> Pedro Alves
>