[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 2/3] watchdog.h: Drop local redefinition of a
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v4 2/3] watchdog.h: Drop local redefinition of actions enum |
Date: |
Wed, 06 Sep 2017 19:25:58 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
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.
Re: [Qemu-devel] [PATCH v4 0/3] watchdog: Allow setting action on the fly, Markus Armbruster, 2017/09/06