[4/9] runstate: create runstate_index function

Given a string state, we need a way to get the RunState for that string.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/sysemu/sysemu.h |  1 +
 vl.c                    | 13 +++++++++++++
 2 files changed, 14 insertions(+)

* Juan Quintela (quintela@redhat.com) wrote:
> Given a string state, we need a way to get the RunState for that string.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  include/sysemu/sysemu.h |  1 +
>  vl.c                    | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index c1a403e..e5fff07 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -29,6 +29,7 @@ void runstate_set(RunState new_state);
>  int runstate_is_running(void);
>  bool runstate_needs_reset(void);
>  int runstate_store(char *str, int size);
> +RunState runstate_index(char *str);
>  typedef struct vm_change_state_entry VMChangeStateEntry;
>  typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
> 
> diff --git a/vl.c b/vl.c
> index 7dca13f..ea4319d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -620,6 +620,19 @@ int runstate_store(char *str, int size)
>      return 0;
>  }
> 
> +RunState runstate_index(char *str)
> +{
> +    RunState i = 0;
> +
> +    while (i != RUN_STATE_MAX) {
> +        if (strcmp(str, RunState_lookup[i]) == 0) {
> +            return i;
> +        }
> +        i++;
> +    }
> +    return -1;

It doesn't seem right to return -1 for the value of an enum
(which is otherwise used as an index into an array of strings).

Make it return a bool and pass the RunState* as a parameter ?

Dave

> +}
> +
>  static void runstate_init(void)
>  {
>      const RunStateTransition *p;
> -- 
> 2.4.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

On 05/18/2015 03:58 AM, Dr. David Alan Gilbert wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Given a string state, we need a way to get the RunState for that string.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  include/sysemu/sysemu.h |  1 +
>>  vl.c                    | 13 +++++++++++++
>>  2 files changed, 14 insertions(+)
>>

>>
>> +RunState runstate_index(char *str)
>> +{
>> +    RunState i = 0;
>> +
>> +    while (i != RUN_STATE_MAX) {
>> +        if (strcmp(str, RunState_lookup[i]) == 0) {
>> +            return i;
>> +        }
>> +        i++;
>> +    }
>> +    return -1;
> 
> It doesn't seem right to return -1 for the value of an enum
> (which is otherwise used as an index into an array of strings).
> 
> Make it return a bool and pass the RunState* as a parameter ?

Why open-code this, when we already have qapi_enum_parse() in
qapi/qapi-util.c?

Eric Blake <eblake@redhat.com> wrote:
D> On 05/18/2015 03:58 AM, Dr. David Alan Gilbert wrote:
>> * Juan Quintela (quintela@redhat.com) wrote:
>>> Given a string state, we need a way to get the RunState for that string.
>>>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>> ---
>>>  include/sysemu/sysemu.h |  1 +
>>>  vl.c                    | 13 +++++++++++++
>>>  2 files changed, 14 insertions(+)
>>>
>
>>>
>>> +RunState runstate_index(char *str)
>>> +{
>>> +    RunState i = 0;
>>> +
>>> +    while (i != RUN_STATE_MAX) {
>>> +        if (strcmp(str, RunState_lookup[i]) == 0) {
>>> +            return i;
>>> +        }
>>> +        i++;
>>> +    }
>>> +    return -1;
>> 
>> It doesn't seem right to return -1 for the value of an enum
>> (which is otherwise used as an index into an array of strings).
>> 
>> Make it return a bool and pass the RunState* as a parameter ?
>
> Why open-code this, when we already have qapi_enum_parse() in
> qapi/qapi-util.c?

Didn't knew about this function.

Thanks, Juan.

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Given a string state, we need a way to get the RunState for that string.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  include/sysemu/sysemu.h |  1 +
>>  vl.c                    | 13 +++++++++++++
>>  2 files changed, 14 insertions(+)
>> 
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index c1a403e..e5fff07 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -29,6 +29,7 @@ void runstate_set(RunState new_state);
>>  int runstate_is_running(void);
>>  bool runstate_needs_reset(void);
>>  int runstate_store(char *str, int size);
>> +RunState runstate_index(char *str);
>>  typedef struct vm_change_state_entry VMChangeStateEntry;
>>  typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
>> 
>> diff --git a/vl.c b/vl.c
>> index 7dca13f..ea4319d 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -620,6 +620,19 @@ int runstate_store(char *str, int size)
>>      return 0;
>>  }
>> 
>> +RunState runstate_index(char *str)
>> +{
>> +    RunState i = 0;
>> +
>> +    while (i != RUN_STATE_MAX) {
>> +        if (strcmp(str, RunState_lookup[i]) == 0) {
>> +            return i;
>> +        }
>> +        i++;
>> +    }
>> +    return -1;
>
> It doesn't seem right to return -1 for the value of an enum
> (which is otherwise used as an index into an array of strings).
>
> Make it return a bool and pass the RunState* as a parameter ?
>
> Dave
>
>> +}
>> +
>>  static void runstate_init(void)
>>  {
>>      const RunStateTransition *p;
>> -- 
>> 2.4.0
>> 
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Dropped, see Eric suggestion.

Thanks, Juan.