* [RFC] remove set_tfile_traceframe and cur_traceframe_number
@ 2012-06-27 13:48 Yao Qi
2012-07-05 13:28 ` [ping]: " Yao Qi
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Yao Qi @ 2012-06-27 13:48 UTC (permalink / raw)
To: gdb-patches
Hi,
I can't see the necessity to use function set_tfile_traceframe and
variable cur_traceframe_number. IIUC, both set_tfile_traceframe
and cur_traceframe_number are equivalent to remote.c:set_remote_traceframe
and remote.c:remote_traceframe_number. set_remote_traceframe
and remote_traceframe_number are used to switch between traceframe
and live inferior in a lazy mode. However, this requirement doesn't
exists in tfile, because GDB only reads from trace file. This is
the reason I propose to remove them. Regression tested on native
and gdbserver on x86_64-linux. OK to apply?
gdb:
2012-06-27 Yao Qi <yao@codesourcery.com>
* tracepoint.c (set_tfile_traceframe): Removed.
(tfile_trace_find, tfile_fetch_registers): Update callers.
(tfile_xfer_partial, tfile_get_trace_state_variable_value): Likewise.
Remove cur_traceframe_number.
(tfile_open, tfile_trace_find): Likewise.
---
gdb/tracepoint.c | 40 ++--------------------------------------
1 files changed, 2 insertions(+), 38 deletions(-)
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 4445256..398ba6a 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -3631,7 +3631,6 @@ char *trace_filename;
int trace_fd = -1;
off_t trace_frames_offset;
off_t cur_offset;
-int cur_traceframe_number;
int cur_data_size;
int trace_regblock_size;
@@ -3724,8 +3723,6 @@ tfile_open (char *filename, int from_tty)
ts->disconnected_tracing = 0;
ts->circular_buffer = 0;
- cur_traceframe_number = -1;
-
TRY_CATCH (ex, RETURN_MASK_ALL)
{
/* Read through a section of newline-terminated lines that
@@ -4225,28 +4222,6 @@ tfile_get_traceframe_address (off_t tframe_offset)
return addr;
}
-/* Make tfile's selected traceframe match GDB's selected
- traceframe. */
-
-static void
-set_tfile_traceframe (void)
-{
- int newnum;
-
- if (cur_traceframe_number == get_traceframe_number ())
- return;
-
- /* Avoid recursion, tfile_trace_find calls us again. */
- cur_traceframe_number = get_traceframe_number ();
-
- newnum = target_trace_find (tfind_number,
- get_traceframe_number (), 0, 0, NULL);
-
- /* Should not happen. If it does, all bets are off. */
- if (newnum != get_traceframe_number ())
- warning (_("could not set tfile's traceframe"));
-}
-
/* Given a type of search and some parameters, scan the collection of
traceframes in the file looking for a match. When found, return
both the traceframe and tracepoint number, otherwise -1 for
@@ -4263,12 +4238,7 @@ tfile_trace_find (enum trace_find_type type, int num,
off_t offset, tframe_offset;
ULONGEST tfaddr;
- /* Lookups other than by absolute frame number depend on the current
- trace selected, so make sure it is correct on the tfile end
- first. */
- if (type != tfind_number)
- set_tfile_traceframe ();
- else if (num == -1)
+ if (num == -1)
{
if (tpp)
*tpp = -1;
@@ -4327,7 +4297,7 @@ tfile_trace_find (enum trace_find_type type, int num,
*tpp = tpnum;
cur_offset = offset;
cur_data_size = data_size;
- cur_traceframe_number = tfnum;
+
return tfnum;
}
/* Skip past the traceframe's data. */
@@ -4442,8 +4412,6 @@ tfile_fetch_registers (struct target_ops *ops,
if (!trace_regblock_size)
return;
- set_tfile_traceframe ();
-
regs = alloca (trace_regblock_size);
if (traceframe_find_block_type ('R', 0) >= 0)
@@ -4527,8 +4495,6 @@ tfile_xfer_partial (struct target_ops *ops, enum target_object object,
if (readbuf == NULL)
error (_("tfile_xfer_partial: trace file is read-only"));
- set_tfile_traceframe ();
-
if (traceframe_number != -1)
{
int pos = 0;
@@ -4614,8 +4580,6 @@ tfile_get_trace_state_variable_value (int tsvnum, LONGEST *val)
{
int pos;
- set_tfile_traceframe ();
-
pos = 0;
while ((pos = traceframe_find_block_type ('V', pos)) >= 0)
{
--
1.7.7.6
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ping]: [RFC] remove set_tfile_traceframe and cur_traceframe_number
2012-06-27 13:48 [RFC] remove set_tfile_traceframe and cur_traceframe_number Yao Qi
@ 2012-07-05 13:28 ` Yao Qi
2012-07-23 16:07 ` Yao Qi
2012-07-27 14:09 ` Pedro Alves
2 siblings, 0 replies; 6+ messages in thread
From: Yao Qi @ 2012-07-05 13:28 UTC (permalink / raw)
To: gdb-patches
On Wednesday, June 27, 2012 09:47:55 PM Yao Qi wrote:
> 2012-06-27 Yao Qi <yao@codesourcery.com>
>
> * tracepoint.c (set_tfile_traceframe): Removed.
> (tfile_trace_find, tfile_fetch_registers): Update callers.
> (tfile_xfer_partial, tfile_get_trace_state_variable_value):
> Likewise. Remove cur_traceframe_number.
> (tfile_open, tfile_trace_find): Likewise.
ping. http://sourceware.org/ml/gdb-patches/2012-06/msg00830.html
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] remove set_tfile_traceframe and cur_traceframe_number
2012-06-27 13:48 [RFC] remove set_tfile_traceframe and cur_traceframe_number Yao Qi
2012-07-05 13:28 ` [ping]: " Yao Qi
@ 2012-07-23 16:07 ` Yao Qi
2012-07-23 16:39 ` Pedro Alves
2012-07-27 14:09 ` Pedro Alves
2 siblings, 1 reply; 6+ messages in thread
From: Yao Qi @ 2012-07-23 16:07 UTC (permalink / raw)
To: gdb-patches; +Cc: palves
On Wednesday, June 27, 2012 09:47:55 PM Yao Qi wrote:
> I can't see the necessity to use function set_tfile_traceframe and
> variable cur_traceframe_number. IIUC, both set_tfile_traceframe
> and cur_traceframe_number are equivalent to remote.c:set_remote_traceframe
> and remote.c:remote_traceframe_number. set_remote_traceframe
> and remote_traceframe_number are used to switch between traceframe
> and live inferior in a lazy mode. However, this requirement doesn't
> exists in tfile, because GDB only reads from trace file. This is
> the reason I propose to remove them. Regression tested on native
> and gdbserver on x86_64-linux. OK to apply?
The code this patch tries to remove was added by Pedro in this patch,
[unavailable values part 1, 05/17] move traceframe memory reading fallback
to read-only sections to GDB side
http://sourceware.org/ml/gdb-patches/2011-02/msg00136.html
in order to make GDB to read read-only memory from the live target. Pedro's
patch did something similar to both remote target and tfile target. It makes
sense to remote target, because there is a live inferior that GDB can access.
However, it is not necessary for tfile target, because there is no live
inferior at all. IMO, it is correct to remove them in this patch.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] remove set_tfile_traceframe and cur_traceframe_number
2012-07-23 16:07 ` Yao Qi
@ 2012-07-23 16:39 ` Pedro Alves
2012-07-27 8:11 ` Yao Qi
0 siblings, 1 reply; 6+ messages in thread
From: Pedro Alves @ 2012-07-23 16:39 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 07/23/2012 05:07 PM, Yao Qi wrote:
> On Wednesday, June 27, 2012 09:47:55 PM Yao Qi wrote:
>> I can't see the necessity to use function set_tfile_traceframe and
>> variable cur_traceframe_number. IIUC, both set_tfile_traceframe
>> and cur_traceframe_number are equivalent to remote.c:set_remote_traceframe
>> and remote.c:remote_traceframe_number. set_remote_traceframe
>> and remote_traceframe_number are used to switch between traceframe
>> and live inferior in a lazy mode. However, this requirement doesn't
>> exists in tfile, because GDB only reads from trace file. This is
>> the reason I propose to remove them. Regression tested on native
>> and gdbserver on x86_64-linux. OK to apply?
>
> The code this patch tries to remove was added by Pedro in this patch,
>
> [unavailable values part 1, 05/17] move traceframe memory reading fallback
> to read-only sections to GDB side
> http://sourceware.org/ml/gdb-patches/2011-02/msg00136.html
>
> in order to make GDB to read read-only memory from the live target. Pedro's
> patch did something similar to both remote target and tfile target. It makes
> sense to remote target, because there is a live inferior that GDB can access.
> However, it is not necessary for tfile target, because there is no live
> inferior at all. IMO, it is correct to remove them in this patch.
Sorry for the delay. I'll take a look at this very soon. (ping me in a couple
days if you don't hear back).
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] remove set_tfile_traceframe and cur_traceframe_number
2012-07-23 16:39 ` Pedro Alves
@ 2012-07-27 8:11 ` Yao Qi
0 siblings, 0 replies; 6+ messages in thread
From: Yao Qi @ 2012-07-27 8:11 UTC (permalink / raw)
To: gdb-patches; +Cc: Pedro Alves
On Monday, July 23, 2012 05:39:09 PM Pedro Alves wrote:
> Sorry for the delay. I'll take a look at this very soon. (ping me in a
> couple days if you don't hear back).
Pedro,
do you have a chance to have a look?
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] remove set_tfile_traceframe and cur_traceframe_number
2012-06-27 13:48 [RFC] remove set_tfile_traceframe and cur_traceframe_number Yao Qi
2012-07-05 13:28 ` [ping]: " Yao Qi
2012-07-23 16:07 ` Yao Qi
@ 2012-07-27 14:09 ` Pedro Alves
2 siblings, 0 replies; 6+ messages in thread
From: Pedro Alves @ 2012-07-27 14:09 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 06/27/2012 02:47 PM, Yao Qi wrote:
> Hi,
> I can't see the necessity to use function set_tfile_traceframe and
> variable cur_traceframe_number. IIUC, both set_tfile_traceframe
> and cur_traceframe_number are equivalent to remote.c:set_remote_traceframe
> and remote.c:remote_traceframe_number. set_remote_traceframe
> and remote_traceframe_number are used to switch between traceframe
> and live inferior in a lazy mode. However, this requirement doesn't
> exists in tfile, because GDB only reads from trace file. This is
> the reason I propose to remove them. Regression tested on native
> and gdbserver on x86_64-linux. OK to apply?
OK. Thanks.
Nits:
> 2012-06-27 Yao Qi <yao@codesourcery.com>
>
> * tracepoint.c (set_tfile_traceframe): Removed.
"Remove."
> (tfile_trace_find, tfile_fetch_registers): Update callers.
> (tfile_xfer_partial, tfile_get_trace_state_variable_value): Likewise.
> Remove cur_traceframe_number.
This is a global. So should be:
(cur_traceframe_number): Remove.
and should be first thing in the entry.
--
Pedro Alves
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-07-27 14:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-27 13:48 [RFC] remove set_tfile_traceframe and cur_traceframe_number Yao Qi
2012-07-05 13:28 ` [ping]: " Yao Qi
2012-07-23 16:07 ` Yao Qi
2012-07-23 16:39 ` Pedro Alves
2012-07-27 8:11 ` Yao Qi
2012-07-27 14:09 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox