| Message ID | 1433878062-23560-1-git-send-email-patrick@parcs.ath.cx |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 66617 invoked by alias); 9 Jun 2015 19:28:26 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 66607 invoked by uid 89); 9 Jun 2015 19:28:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL, BAYES_40, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-qg0-f43.google.com Received: from mail-qg0-f43.google.com (HELO mail-qg0-f43.google.com) (209.85.192.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 09 Jun 2015 19:28:24 +0000 Received: by qgf75 with SMTP id 75so8952058qgf.1 for <gdb-patches@sourceware.org>; Tue, 09 Jun 2015 12:28:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=5JANbEz12NDacQ1sdHp43tNOYf4DXhd47w9MlqxDm2s=; b=DC0LrgAtfb+mxhF6MUbHsXWQoDYe97zaLuz6LOkUeWqbKctnmy0V3SS4csC/hZCEUN qq4JKtYRzAsl/i9kriD7PU3YHtOS++khwEGjCUHPn3XXUr1sDrcGCXbwag4X+1kONlpu FZdpND32pCM2CLsOTGqaEWW3z5X7pNi3vo/0JnoX4uSzRXs77GRp3Ed3ha224xMFlyN1 dXo1BfalxQ9ZLM3Et9lazke+4HyKLKXFFFPyPJSHIPbDHhaTLliwFMrQqvWDwQZcUIR1 5Ji1guBjr28lAHHS7CM60ytXkBywpeJESFSzdNjcdOHejPg7F3HrBbhKfrQ5YPw9Hivj XwOA== X-Gm-Message-State: ALoCoQnPp7FV38SVAyjyXglTN3DygZEUkap0YmcN1Ela7B/zzb5WL+cuwWvgiF4MQ6g+lq5Es7HU X-Received: by 10.55.24.209 with SMTP id 78mr46673764qky.19.1433878102491; Tue, 09 Jun 2015 12:28:22 -0700 (PDT) Received: from localhost.localdomain (ool-4353acd8.dyn.optonline.net. [67.83.172.216]) by mx.google.com with ESMTPSA id t77sm3093653qgt.42.2015.06.09.12.28.21 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 09 Jun 2015 12:28:21 -0700 (PDT) From: Patrick Palka <patrick@parcs.ath.cx> To: gdb-patches@sourceware.org Cc: Patrick Palka <patrick@parcs.ath.cx> Subject: [PATCH] Don't truncate the history file when history size is unlimited Date: Tue, 9 Jun 2015 15:27:42 -0400 Message-Id: <1433878062-23560-1-git-send-email-patrick@parcs.ath.cx> |
Commit Message
Patrick Palka
June 9, 2015, 7:27 p.m. UTC
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. 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. 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. (Apparently I did not test my changes to history file handling extensively enough. Sorry..) gdb/ChangeLog: * top.c (gdb_safe_append_history): Do not call history_truncate_file if the history is not stifled. --- gdb/top.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Comments
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? > 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. > (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
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 >
On 06/15/2015 05:00 PM, Patrick Palka wrote: > 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? Use [standard_output_file $some_file]. E.g., take a look at gdb.trace/tfile.exp, gdb.xml/tdesc-arch.exp, gdb.xml/tdesc-regs.exp and others. Thanks, Pedro Alves
diff --git a/gdb/top.c b/gdb/top.c index 837bf16..f5a0819 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -981,7 +981,8 @@ gdb_safe_append_history (void) else { append_history (command_count, local_history_filename); - history_truncate_file (local_history_filename, history_max_entries); + if (history_is_stifled ()) + history_truncate_file (local_history_filename, history_max_entries); } ret = rename (local_history_filename, history_filename);