Hi,
In OpenOCD, all gdb packet handling functions look like:
gdb_xxx_packet(connection, target, packet, packet_size);
I'm wondering if we can remove target from the argument list and just
calculate it from connection in each handling function. The patch is
attached.
I like this patch since I think the code is clearer with it and the
big switch in gdb_input_inner will be more generic. The cons is the
code size will be a little larger, on x86_64, it's from 1295167B to
1295343B. But I think it's not a real issue. What's your opinion?
Regards,
Jie
From 9d7c77df0ba54d659bc995b997d74411f433eb3e Mon Sep 17 00:00:00 2001
From: Jie Zhang <jie.zh...@analog.com>
Date: Tue, 23 Aug 2011 17:29:29 -0400
Subject: [PATCH] remove target argument from gdb packet handling functions
---
src/rtos/rtos.c | 10 +++-
src/rtos/rtos.h | 4 +-
src/server/gdb_server.c | 118 +++++++++++++++++++++++------------------------
src/target/smp.c | 8 ++-
src/target/smp.h | 4 +-
5 files changed, 76 insertions(+), 68 deletions(-)
diff --git a/src/rtos/rtos.c b/src/rtos/rtos.c
index 263795c..12a944f 100644
--- a/src/rtos/rtos.c
+++ b/src/rtos/rtos.c
@@ -128,8 +128,11 @@ int rtos_create(Jim_GetOptInfo *goi, struct target * target)
-int gdb_thread_packet(struct connection *connection, struct target *target, char *packet, int packet_size)
+int gdb_thread_packet(struct connection *connection, char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
+
if (strstr(packet, "qP"))
{
#define TAG_THREADID 1 /* Echo the thread identifier */
@@ -501,8 +504,11 @@ int gdb_thread_packet(struct connection *connection, struct target *target, char
return GDB_THREAD_PACKET_NOT_CONSUMED;
}
-int rtos_get_gdb_reg_list(struct connection *connection, struct target *target, struct reg **reg_list[], int *reg_list_size)
+int rtos_get_gdb_reg_list(struct connection *connection, struct reg **reg_list[], int *reg_list_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
+
if ( ( target->rtos != NULL ) &&
( current_threadid != -1 ) &&
( current_threadid != 0 ) &&
diff --git a/src/rtos/rtos.h b/src/rtos/rtos.h
index a6378c6..1a73bd7 100644
--- a/src/rtos/rtos.h
+++ b/src/rtos/rtos.h
@@ -99,8 +99,8 @@ struct rtos_register_stacking
int rtos_create(Jim_GetOptInfo *goi, struct target * target);
int rtos_generic_stack_read( struct target * target, const struct rtos_register_stacking* stacking, int64_t stack_ptr, char ** hex_reg_list );
int rtos_try_next( struct target * target );
-int gdb_thread_packet(struct connection *connection, struct target *target, char *packet, int packet_size);
-int rtos_get_gdb_reg_list(struct connection *connection, struct target *target, struct reg **reg_list[], int *reg_list_size);
+int gdb_thread_packet(struct connection *connection, char *packet, int packet_size);
+int rtos_get_gdb_reg_list(struct connection *connection, struct reg **reg_list[], int *reg_list_size);
int rtos_update_threads( struct target *target );
#endif // RTOS_H
diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index b6921ff..3509b93 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -944,8 +944,10 @@ static void gdb_send_error(struct connection *connection, uint8_t the_error)
}
static int gdb_last_signal_packet(struct connection *connection,
- struct target *target, char* packet, int packet_size)
+ char* packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
char sig_reply[4];
int signal_var;
@@ -1029,8 +1031,10 @@ static void gdb_target_to_reg(struct target *target,
}
static int gdb_get_registers_packet(struct connection *connection,
- struct target *target, char* packet, int packet_size)
+ char* packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
struct reg **reg_list;
int reg_list_size;
int retval;
@@ -1044,7 +1048,7 @@ static int gdb_get_registers_packet(struct connection *connection,
#endif
if ( ( target->rtos != NULL ) &&
- ( ERROR_FAIL != rtos_get_gdb_reg_list( connection, target, ®_list, ®_list_size) ) )
+ ( ERROR_FAIL != rtos_get_gdb_reg_list( connection, ®_list, ®_list_size) ) )
{
return ERROR_OK;
}
@@ -1088,8 +1092,10 @@ static int gdb_get_registers_packet(struct connection *connection,
}
static int gdb_set_registers_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
int i;
struct reg **reg_list;
int reg_list_size;
@@ -1147,8 +1153,10 @@ static int gdb_set_registers_packet(struct connection *connection,
}
static int gdb_get_register_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
char *reg_packet;
int reg_num = strtoul(packet + 1, NULL, 16);
struct reg **reg_list;
@@ -1186,8 +1194,10 @@ static int gdb_get_register_packet(struct connection *connection,
}
static int gdb_set_register_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
char *separator;
uint8_t *bin_buf;
int reg_num = strtoul(packet + 1, &separator, 16);
@@ -1249,8 +1259,10 @@ static int gdb_error(struct connection *connection, int retval)
* 8191 bytes by the looks of it. Why 8191 bytes instead of 8192?????
*/
static int gdb_read_memory_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
char *separator;
uint32_t addr = 0;
uint32_t len = 0;
@@ -1324,8 +1336,10 @@ static int gdb_read_memory_packet(struct connection *connection,
}
static int gdb_write_memory_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
char *separator;
uint32_t addr = 0;
uint32_t len = 0;
@@ -1382,8 +1396,10 @@ static int gdb_write_memory_packet(struct connection *connection,
}
static int gdb_write_memory_binary_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
char *separator;
uint32_t addr = 0;
uint32_t len = 0;
@@ -1446,8 +1462,10 @@ static int gdb_write_memory_binary_packet(struct connection *connection,
}
static int gdb_step_continue_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
int current = 0;
uint32_t address = 0x0;
int retval = ERROR_OK;
@@ -1480,8 +1498,10 @@ static int gdb_step_continue_packet(struct connection *connection,
}
static int gdb_breakpoint_watchpoint_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
int type;
enum breakpoint_type bp_type = BKPT_SOFT /* dummy init to avoid warning */;
enum watchpoint_rw wp_type = WPT_READ /* dummy init to avoid warning */;
@@ -1674,7 +1694,7 @@ static int compare_bank (const void * a, const void * b)
}
static int gdb_memory_map(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
/* We get away with only specifying flash here. Regions that are not
* specified are treated as if we provided no memory map(if not we
@@ -1683,6 +1703,8 @@ static int gdb_memory_map(struct connection *connection,
* have to regenerate it a couple of times.
*/
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
struct flash_bank *p;
char *xml = NULL;
int size = 0;
@@ -1818,10 +1840,12 @@ static int gdb_memory_map(struct connection *connection,
}
static int gdb_query_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
struct command_context *cmd_ctx = connection->cmd_ctx;
struct gdb_connection *gdb_connection = connection->priv;
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
if (strstr(packet, "qRcmd,"))
{
@@ -1919,7 +1943,7 @@ static int gdb_query_packet(struct connection *connection,
}
else if (strstr(packet, "qXfer:memory-map:read::")
&& (flash_get_bank_count() > 0))
- return gdb_memory_map(connection, target, packet, packet_size);
+ return gdb_memory_map(connection, packet, packet_size);
else if (strstr(packet, "qXfer:features:read:"))
{
char *xml = NULL;
@@ -1972,7 +1996,7 @@ static int gdb_query_packet(struct connection *connection,
}
static int gdb_v_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
struct gdb_connection *gdb_connection = connection->priv;
struct gdb_service *gdb_service = connection->service->priv;
@@ -2119,7 +2143,7 @@ static int gdb_v_packet(struct connection *connection,
return ERROR_OK;
}
-static int gdb_detach(struct connection *connection, struct target *target)
+static int gdb_detach(struct connection *connection)
{
struct gdb_service *gdb_service = connection->service->priv;
@@ -2207,61 +2231,43 @@ static int gdb_input_inner(struct connection *connection)
switch (packet[0])
{
case 'T': // Is thread alive?
- gdb_thread_packet(connection, target, packet, packet_size);
+ gdb_thread_packet(connection, packet, packet_size);
break;
case 'H': // Set current thread ( 'c' for step and continue, 'g' for all other operations )
- gdb_thread_packet(connection, target, packet, packet_size);
+ gdb_thread_packet(connection, packet, packet_size);
break;
case 'q':
case 'Q':
- retval = gdb_thread_packet(connection,
- target, packet,
- packet_size);
+ retval = gdb_thread_packet(connection, packet, packet_size);
if ( retval == GDB_THREAD_PACKET_NOT_CONSUMED )
{
- retval = gdb_query_packet(connection,
- target, packet,
- packet_size);
+ retval = gdb_query_packet(connection, packet, packet_size);
}
break;
case 'g':
- retval = gdb_get_registers_packet(
- connection, target,
- packet, packet_size);
+ retval = gdb_get_registers_packet(connection, packet, packet_size);
break;
case 'G':
- retval = gdb_set_registers_packet(
- connection, target,
- packet, packet_size);
+ retval = gdb_set_registers_packet(connection, packet, packet_size);
break;
case 'p':
- retval = gdb_get_register_packet(
- connection, target,
- packet, packet_size);
+ retval = gdb_get_register_packet(connection, packet, packet_size);
break;
case 'P':
- retval = gdb_set_register_packet(
- connection, target,
- packet, packet_size);
+ retval = gdb_set_register_packet(connection, packet, packet_size);
break;
case 'm':
- retval = gdb_read_memory_packet(
- connection, target,
- packet, packet_size);
+ retval = gdb_read_memory_packet(connection, packet, packet_size);
break;
case 'M':
- retval = gdb_write_memory_packet(
- connection, target,
- packet, packet_size);
+ retval = gdb_write_memory_packet(connection, packet, packet_size);
break;
case 'z':
case 'Z':
- retval = gdb_breakpoint_watchpoint_packet(connection, target, packet, packet_size);
+ retval = gdb_breakpoint_watchpoint_packet(connection, packet, packet_size);
break;
case '?':
- gdb_last_signal_packet(
- connection, target,
- packet, packet_size);
+ gdb_last_signal_packet(connection, packet, packet_size);
break;
case 'c':
case 's':
@@ -2324,7 +2330,7 @@ static int gdb_input_inner(struct connection *connection)
{
/* Here we don't want packet processing to stop even if this fails,
* so we use a local variable instead of retval. */
- retval = gdb_step_continue_packet(connection, target, packet, packet_size);
+ retval = gdb_step_continue_packet(connection, packet, packet_size);
if (retval != ERROR_OK)
{
/* we'll never receive a halted condition... issue a false one.. */
@@ -2335,18 +2341,14 @@ static int gdb_input_inner(struct connection *connection)
}
break;
case 'v':
- retval = gdb_v_packet(
- connection, target,
- packet, packet_size);
+ retval = gdb_v_packet(connection, packet, packet_size);
break;
case 'D':
- retval = gdb_detach(connection, target);
+ retval = gdb_detach(connection);
extended_protocol = 0;
break;
case 'X':
- retval = gdb_write_memory_binary_packet(
- connection, target,
- packet, packet_size);
+ retval = gdb_write_memory_binary_packet(connection, packet, packet_size);
if (retval != ERROR_OK)
return retval;
break;
@@ -2373,18 +2375,14 @@ static int gdb_input_inner(struct connection *connection)
case 'j':
/* packet supported only by smp target i.e cortex_a.c*/
/* handle smp packet replying coreid played to gbd */
- gdb_read_smp_packet(
- connection, target,
- packet, packet_size);
+ gdb_read_smp_packet(connection, packet, packet_size);
break;
case 'J':
/* packet supported only by smp target i.e cortex_a.c */
/* handle smp packet setting coreid to be played at next
* resume to gdb */
- gdb_write_smp_packet(
- connection, target,
- packet, packet_size);
+ gdb_write_smp_packet(connection, packet, packet_size);
break;
default:
diff --git a/src/target/smp.c b/src/target/smp.c
index ec157d3..c42d921 100644
--- a/src/target/smp.c
+++ b/src/target/smp.c
@@ -59,8 +59,10 @@ static const char DIGITS[16] = "0123456789abcdef";
/* packet j :smp status request */
int gdb_read_smp_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
uint32_t len = sizeof(int32_t);
uint8_t *buffer;
char *hex_buffer;
@@ -91,8 +93,10 @@ int gdb_read_smp_packet(struct connection *connection,
/* J : smp set request */
int gdb_write_smp_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size)
+ char *packet, int packet_size)
{
+ struct gdb_service *gdb_service = connection->service->priv;
+ struct target *target = gdb_service->target;
char *separator;
int coreid = 0;
int retval = ERROR_OK;
diff --git a/src/target/smp.h b/src/target/smp.h
index f85c9a4..b8f0ea5 100644
--- a/src/target/smp.h
+++ b/src/target/smp.h
@@ -19,7 +19,7 @@
***************************************************************************/
#include "server/server.h"
int gdb_read_smp_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size);
+ char *packet, int packet_size);
int gdb_write_smp_packet(struct connection *connection,
- struct target *target, char *packet, int packet_size);
+ char *packet, int packet_size);
--
1.7.5.4
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
