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

[Glauber Costa]

On Thu, Oct 23, 2008 at 09:03:45AM -0500, Anthony Liguori wrote:
> Glauber Costa wrote:
>> From: Glauber Costa <address@hidden>
>>
>> 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 <address@hidden>
>> ---
>>  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?
ok, this makes sense.

thanks.
>
>> +    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