Re: [Qemu-devel] [PATCH v4 2/3] watchdog.h: Drop local redefinition of actions enum

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 09/06/2017 06:24 AM, Michal Privoznik wrote:
>> We already have enum that enumerates all the action that a
>
> s/action/actions/
>
>> watchdog can take when hitting its timeout: WatchdogAction.
>> Use that instead of inventing our own.
>> 
>> Signed-off-by: Michal Privoznik <address@hidden>
>> ---
>
>> @@ -77,27 +77,16 @@ int select_watchdog(const char *p)
>>  
>>  int select_watchdog_action(const char *p)
>>  {
>> -    if (strcasecmp(p, "reset") == 0)
>> -        watchdog_action = WDT_RESET;
>
> The old code was case-insensitive,
>
>> +    action = qapi_enum_parse(&WatchdogAction_lookup, p, -1, NULL);
>
> the new code is not.  Do we care?  (I don't, but we could be breaking
> someone's control flow).  Should qapi_enum_parse be taught to be
> case-insensitive?  Or perhaps we answer related questions first: Do we
> have any QAPI enums that have values differing only in case? Do we
> prevent such QAPI definitions, to give us the potential of making the
> parsing insensitive?

Case-sensitive everywhere is fine.  Case-insensitive everywhere also
fine, just not my personal preference.  What's not fine is "guess
whether this part of the interface is case-sensitive or not".

QMP is case-sensitive.  Let's keep it that way.

The -watchdog-action option has a case-insensitive argument.  The
obvious way to remain misfeature-^Wbackwards compatible is converting
the argument to lower case before handing it off to qapi_enum_parse.  I
doubt it matters, but just doing it is less work than debating how far
exactly we want to bend over backwards.

g_ascii_strdown() should do.  It only converts ASCII characters, but
anything else is going to fail in qapi_enum_parse() anyway.