Nicolas Eder <nicolas.eder@lauterbach.com> writes:
> ---
> gdbstub/gdbstub.c | 9 +--------
> gdbstub/internals.h | 26 --------------------------
> include/cutils.h | 30 ++++++++++++++++++++++++++++++
> include/exec/gdbstub.h | 9 ++++++++-
Please split into the utils and the reg state patches as they are unrelated.
> 4 files changed, 39 insertions(+), 35 deletions(-)
> create mode 100644 include/cutils.h
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 1e96a71c0c..c43ff89393 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -44,14 +44,7 @@
> #include "exec/hwaddr.h"
>
> #include "internals.h"
> -
> -typedef struct GDBRegisterState {
> - int base_reg;
> - int num_regs;
> - gdb_get_reg_cb get_reg;
> - gdb_set_reg_cb set_reg;
> - const char *xml;
> -} GDBRegisterState;
> +#include "cutils.h"
>
> GDBState gdbserver_state;
>
> diff --git a/gdbstub/internals.h b/gdbstub/internals.h
> index 465c24b36e..011e6f1858 100644
> --- a/gdbstub/internals.h
> +++ b/gdbstub/internals.h
> @@ -74,32 +74,6 @@ typedef struct GDBState {
> /* lives in main gdbstub.c */
> extern GDBState gdbserver_state;
>
> -/*
> - * Inline utility function, convert from int to hex and back
> - */
> -
> -static inline int fromhex(int v)
> -{
> - if (v >= '0' && v <= '9') {
> - return v - '0';
> - } else if (v >= 'A' && v <= 'F') {
> - return v - 'A' + 10;
> - } else if (v >= 'a' && v <= 'f') {
> - return v - 'a' + 10;
> - } else {
> - return 0;
> - }
> -}
> -
> -static inline int tohex(int v)
> -{
> - if (v < 10) {
> - return v + '0';
> - } else {
> - return v - 10 + 'a';
> - }
> -}
> -
> /*
> * Connection helpers for both system and user backends
> */
> diff --git a/include/cutils.h b/include/cutils.h
> new file mode 100644
> index 0000000000..a6b8dc3690
> --- /dev/null
> +++ b/include/cutils.h
> @@ -0,0 +1,30 @@
> +#ifndef CUTILS_H
> +#define CUTILS_H
We already have include/qemu/cutils.h where I think these can go.
> +
> +/*
> + * Inline utility function, convert from int to hex and back
> + */
Becoming common util functions they could do with a little cleaning up
before wider use.
- kdoc comment string
- rename to be clearer, maybe:
- hexchar_to_nibble
- nibble_to_hexchar
> +
> +static inline int fromhex(int v)
> +{
> + if (v >= '0' && v <= '9') {
> + return v - '0';
> + } else if (v >= 'A' && v <= 'F') {
> + return v - 'A' + 10;
> + } else if (v >= 'a' && v <= 'f') {
> + return v - 'a' + 10;
> + } else {
> + return 0;
g_assert_not_reached()? or document invalid chars are squashed to 0
> + }
> +}
> +
> +static inline int tohex(int v)
> +{
g_assert(v =< 0xf)
> + if (v < 10) {
> + return v + '0';
> + } else {
> + return v - 10 + 'a';
> + }
> +}
> +
> +#endif /* CUTILS_H */
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index f696e29477..cb70ebd7b4 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -15,11 +15,18 @@ typedef struct GDBFeature {
> const char *xml;
> } GDBFeature;
>
> -
> /* Get or set a register. Returns the size of the register. */
> typedef int (*gdb_get_reg_cb)(CPUArchState *env, GByteArray *buf, int reg);
> typedef int (*gdb_set_reg_cb)(CPUArchState *env, uint8_t *buf, int reg);
>
> +typedef struct GDBRegisterState {
> + int base_reg;
> + int num_regs;
> + gdb_get_reg_cb get_reg;
> + gdb_set_reg_cb set_reg;
> + const char *xml;
> +} GDBRegisterState;
> +
> /**
> * gdb_register_coprocessor() - register a supplemental set of registers
> * @cpu - the CPU associated with registers
--
Alex Bennée
Virtualisation Tech Lead @ Linaro