[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/4] keyval: Parse help options
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v2 1/4] keyval: Parse help options |
Date: |
Thu, 1 Oct 2020 13:33:51 +0200 |
Am 01.10.2020 um 12:34 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
>
> > Am 30.09.2020 um 15:35 hat Eric Blake geschrieben:
> >> On 9/30/20 7:45 AM, Kevin Wolf wrote:
> >> > This adds a new parameter 'help' to keyval_parse() that enables parsing
> >> > of help options. If NULL is passed, the function behaves the same as
> >> > before. But if a bool pointer is given, it contains the information
> >> > whether an option "help" without value was given (which would otherwise
> >> > either result in an error or be interpreted as the value for an implied
> >> > key).
> >> >
> >> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> [...]
> >> > +++ b/util/keyval.c
> >> > @@ -166,7 +166,7 @@ static QObject *keyval_parse_put(QDict *cur,
> >> > * On failure, return NULL.
> >> > */
> >> > static const char *keyval_parse_one(QDict *qdict, const char *params,
> >> > - const char *implied_key,
> >> > + const char *implied_key, bool *help,
> >> > Error **errp)
> >> > {
> >> > const char *key, *key_end, *s, *end;
> >> > @@ -238,13 +238,20 @@ static const char *keyval_parse_one(QDict *qdict,
> >> > const char *params,
> >> > if (key == implied_key) {
> >> > assert(!*s);
> >> > s = params;
> >> > + } else if (*s == '=') {
> >> > + s++;
> >> > } else {
> >> > - if (*s != '=') {
> >> > + if (help && !strncmp(key, "help", s - key)) {
> >>
> >> Should this use is_help_option() to also accept "?", or are we okay
> >> demanding exactly "help"?
> >
> > The comment for is_help_option() calls "?" deprecated, so I think we
> > don't want to enable it in a new parser.
>
> Valid point.
>
> But do we really want comparisons for "help" inline everywhere we want
> to check for non-deprecated help requests? Would a common helper
> function be better?
How many more parsers are we going to add? I thought we intended the
keyval parser to be the one canoncial place in the long term?
If you prefer, I can make this a second static inline function in
include/qemu/help_option.h (that would have to work both as strcmp and
as strncmp), but I don't see a second user that would make it a "common"
helper.
> On the deprecation of "?": the comment is more than eight years old
> (commit c8057f951d6). We didn't have a deprecation process back then,
> but we did purge '?' from the documentation. Can we finally drop it?
> I'm going to ask that in a new thread.
I guess we can.
Kevin