[Qemu-devel] Re: [PATCH 10/32] turn info kqemu into generic info accelerator

[Anthony Liguori <aliguori@us.ibm.com>]

Glauber Costa wrote:
> From: Glauber Costa <gcosta@redhat.com>
>
> Yet another accel field: info.
> From this point on, "info kqemu" is no more. "info accelerator" should
> be used instead.
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  accel.c   |    6 ++++++
>  accel.h   |    8 ++++++++
>  kqemu.c   |   26 ++++++++++++++++++++++++++
>  monitor.c |   35 ++++++++++++-----------------------
>  4 files changed, 52 insertions(+), 23 deletions(-)
>
> diff --git a/accel.c b/accel.c
> index 6776244..cb615d7 100644
> --- a/accel.c
> +++ b/accel.c
> @@ -8,6 +8,11 @@ int _accel_nop(void)
>      return 0;
>  }
>
> +int noaccel_info(CPUState *env, char *buf)
> +{
> +    return snprintf(buf, MAX_INFO_BUF, "no accelerator present.\n");
> +}
> +
>  #define accel_nop ((void *)_accel_nop)
>
>  /* Accelerator wrapper for the no-accel (raw qemu) case */
> @@ -16,5 +21,6 @@ QEMUAccel noaccel = {
>      .init_env = accel_nop,
>      .flush_cache = accel_nop,
>      .flush_page = accel_nop,
> +    .info = noaccel_info,
>  };
>
> diff --git a/accel.h b/accel.h
> index 935cfef..549ce01 100644
> --- a/accel.h
> +++ b/accel.h
> @@ -1,11 +1,14 @@
>  #ifndef _ACCEL_H_
>  #define _ACCEL_H_
>
> +#define MAX_INFO_BUF 1024
> +
>  typedef struct QEMUAccel {
>      void (*cpu_interrupt)(CPUState *env);
>      void (*init_env)(CPUState *env);
>      void (*flush_cache)(CPUState *env, int global);
>      void (*flush_page)(CPUState *env, target_ulong addr);
> +    int (*info)(CPUState *env, char *buf);
>  } QEMUAccel;
>
>  extern QEMUAccel *current_accel;
> @@ -35,4 +38,9 @@ static inline void accel_flush_page(CPUState *env, target_ulong addr)
>  {
>      current_accel->flush_page(env, addr);
>  }
> +
> +static inline int accel_info(CPUState *env, char *buf)
> +{
> +    return current_accel->info(env, buf);
> +}
>  #endif
> diff --git a/kqemu.c b/kqemu.c
> index 3f2433a..424d8f4 100644
> --- a/kqemu.c
> +++ b/kqemu.c
> @@ -1047,11 +1047,37 @@ static void qpi_init(void)
>                                   0x1000, qpi_io_memory);
>  }
>
> +static int kqemu_info(CPUState *env, char *buf)
> +{
> +    int val, len;
> +    int bufsiz = MAX_INFO_BUF;
>   

Why not just pass bufsiz as an argument to kqemu_info?

> +    if (accel_info(env, buf))
> +        term_printf(buf);
>   

You should do term_printf("%s", buf);  This is a common exploit if 
there's ever a chance that buf has user-originated data.  Therefore, 
it's good practice to always use ("%s", buf) instead of passing buf 
directly.

Regards,

Anthony Liguori