Re: [PATCH v2 7/8] monitor: Reduce target-specific methods further

["Dr. David Alan Gilbert" <dave@treblig.org>]

* Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
> get_monitor_def() doesn't use any target-specific declaration
> anymore, move it to hmp.c to compile it once.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>

> ---
>  monitor/monitor-internal.h |  1 -
>  monitor/hmp-target.c       | 49 -----------------------------------
>  monitor/hmp.c              | 52 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+), 50 deletions(-)
> 
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index 8dc88963630..3ecd394ecf6 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -181,7 +181,6 @@ void monitor_data_destroy_qmp(MonitorQMP *mon);
>  void coroutine_fn monitor_qmp_dispatcher_co(void *data);
>  void qmp_dispatcher_co_wake(void);
>  
> -int get_monitor_def(Monitor *mon, uint64_t *pval, const char *name);
>  void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
>  int hmp_compare_cmd(const char *name, const char *list);
>  
> diff --git a/monitor/hmp-target.c b/monitor/hmp-target.c
> index 01be0a0b426..a3306b69c93 100644
> --- a/monitor/hmp-target.c
> +++ b/monitor/hmp-target.c
> @@ -23,7 +23,6 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "qemu/bswap.h"
>  #include "monitor-internal.h"
>  #include "monitor/qdev.h"
>  #include "net/slirp.h"
> @@ -60,54 +59,6 @@ HMPCommand *hmp_cmds_for_target(bool info_command)
>      return info_command ? hmp_info_cmds : hmp_cmds;
>  }
>  
> -/*
> - * Set @pval to the value in the register identified by @name.
> - * return 0 if OK, -1 if not found
> - */
> -int get_monitor_def(Monitor *mon, uint64_t *pval, const char *name)
> -{
> -    const unsigned length = target_long_bits() / 8;
> -    const MonitorDef *md = target_monitor_defs();
> -    CPUState *cs = mon_get_cpu(mon);
> -    uint64_t tmp = 0;
> -    int ret;
> -
> -    if (cs == NULL || md == NULL) {
> -        return -1;
> -    }
> -
> -    for(; md->name != NULL; md++) {
> -        if (hmp_compare_cmd(name, md->name)) {
> -            if (md->get_value) {
> -                *pval = md->get_value(mon, md, md->offset);
> -            } else {
> -                CPUArchState *env = mon_get_cpu_env(mon);
> -                void *ptr = (uint8_t *)env + md->offset;
> -
> -                switch(md->type) {
> -                case MD_I32:
> -                    *pval = *(uint32_t *)ptr;
> -                    break;
> -                case MD_TLONG:
> -                    *pval = ldn_he_p(ptr, length);
> -                    break;
> -                default:
> -                    *pval = 0;
> -                    break;
> -                }
> -            }
> -            return 0;
> -        }
> -    }
> -
> -    ret = target_get_monitor_def(cs, name, &tmp);
> -    if (!ret) {
> -        *pval = ldn_he_p(&tmp, length);
> -    }
> -
> -    return ret;
> -}
> -
>  static int
>  compare_mon_cmd(const void *a, const void *b)
>  {
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index eee8b7e964e..82d2bbdf77d 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -27,8 +27,10 @@
>  #include "hw/core/qdev.h"
>  #include "monitor-internal.h"
>  #include "monitor/hmp.h"
> +#include "monitor/hmp-target.h"
>  #include "qobject/qdict.h"
>  #include "qobject/qnum.h"
> +#include "qemu/bswap.h"
>  #include "qemu/config-file.h"
>  #include "qemu/ctype.h"
>  #include "qemu/cutils.h"
> @@ -311,6 +313,8 @@ void hmp_help_cmd(Monitor *mon, const char *name)
>  static const char *pch;
>  static sigjmp_buf expr_env;
>  
> +static int get_monitor_def(Monitor *mon, uint64_t *pval, const char *name);
> +
>  static G_NORETURN G_GNUC_PRINTF(2, 3)
>  void expr_error(Monitor *mon, const char *fmt, ...)
>  {
> @@ -1552,3 +1556,51 @@ void monitor_register_hmp_info_hrt(const char *name,
>      }
>      g_assert_not_reached();
>  }
> +
> +/*
> + * Set @pval to the value in the register identified by @name.
> + * return 0 if OK, -1 if not found
> + */
> +static int get_monitor_def(Monitor *mon, uint64_t *pval, const char *name)
> +{
> +    const unsigned length = target_long_bits() / 8;
> +    const MonitorDef *md = target_monitor_defs();
> +    CPUState *cs = mon_get_cpu(mon);
> +    uint64_t tmp = 0;
> +    int ret;
> +
> +    if (cs == NULL || md == NULL) {
> +        return -1;
> +    }
> +
> +    for(; md->name != NULL; md++) {
> +        if (hmp_compare_cmd(name, md->name)) {
> +            if (md->get_value) {
> +                *pval = md->get_value(mon, md, md->offset);
> +            } else {
> +                CPUArchState *env = mon_get_cpu_env(mon);
> +                void *ptr = (uint8_t *)env + md->offset;
> +
> +                switch(md->type) {
> +                case MD_I32:
> +                    *pval = *(uint32_t *)ptr;
> +                    break;
> +                case MD_TLONG:
> +                    *pval = ldn_he_p(ptr, length);
> +                    break;
> +                default:
> +                    *pval = 0;
> +                    break;
> +                }
> +            }
> +            return 0;
> +        }
> +    }
> +
> +    ret = target_get_monitor_def(cs, name, &tmp);
> +    if (!ret) {
> +        *pval = ldn_he_p(&tmp, length);
> +    }
> +
> +    return ret;
> +}
> -- 
> 2.52.0
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/