On 05/30/2012 12:51 PM, Jan Kiszka wrote:
On 2012-05-30 00:10, Igor Mammedov wrote:
current callers all do the same thing, storing in prev_debug_excp_handler
previous handler and then calling it in breakpoint_handler.
Move prev_debug_excp_handler from local scope to global and make
cpu_set_debug_excp_handler() always to store previous handler.
NAK, this is not a beautiful interface, exposing the previous handler
via global variable. And it prevents chaining of handlers.
Is there a technical reason for this refactoring?
current 2 users use prev_debug_excp_handler in the same manner i.e. doing
prev_debug_excp_handler =
cpu_set_debug_excp_handler(breakpoint_handler);
so consolidating it in one place looks better than writing the same code
per-target.
In addition when chaining more then current and previous handlers would be
needed,
the global var prev_debug_excp_handler could be replaced by global call
prev_debug_excp_handler() without affecting per-target code and it could be
implemented in cpu-exec.c or even doing chaining completely internally in
cpu-exec.c
without breakpoint_handler() being aware of it.
PS:
For moving tcg initialization from helper.c into cpu.c I would need to make
prev_debug_excp_handler global per target any way for above assignment to
compile and it would be the same ugliness but without possible future.
PS2:
alternative with defining tcg_x86_init() call inside helper.c and calling
from cpu.c is acceptable too, so if you still don't like idea of this patch
I can go this way.
Jan
Signed-off-by: Igor Mammedov<imamm...@redhat.com>
---
cpu-exec.c | 7 +++----
exec-all.h | 3 ++-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index 83cac93..44c83d9 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -155,13 +155,12 @@ static inline TranslationBlock *tb_find_fast(CPUArchState
*env)
}
static CPUDebugExcpHandler *debug_excp_handler;
+CPUDebugExcpHandler *prev_debug_excp_handler;
-CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler)
+void cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler)
{
- CPUDebugExcpHandler *old_handler = debug_excp_handler;
-
+ prev_debug_excp_handler = debug_excp_handler;
debug_excp_handler = handler;
- return old_handler;
}
static void cpu_handle_debug_exception(CPUArchState *env)
diff --git a/exec-all.h b/exec-all.h
index 9bda7f7..f3c3a8a 100644
--- a/exec-all.h
+++ b/exec-all.h
@@ -357,7 +357,8 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env1,
target_ulong addr);
typedef void (CPUDebugExcpHandler)(CPUArchState *env);
-CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler);
+extern CPUDebugExcpHandler *prev_debug_excp_handler;
+void cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler);
/* vl.c */
extern int singlestep;
--
-----
Igor
