From: Marc-André Lureau <marcandre.lureau@redhat.com>

This allows for a more pleasant user experience.

Before:
$ ./qemu-system-x86_64 -display egl-headless,gl=
qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value ''

After:
$ ./qemu-system-x86_64 -display egl-headless,gl=
qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value ''
Available: 'off', 'on', 'core', 'es'

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qapi/qapi-visit-core.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 6c13510a2b..45e39ff533 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -392,9 +392,25 @@ static bool output_type_enum(Visitor *v, const char *name, int *obj,
     return visit_type_str(v, name, &enum_str, errp);
 }
 
+static void error_append_qapi_enum_hint(Error **errp, const QEnumLookup *lookup)
+{
+    int i;
+
+    error_append_hint(errp, "Available: ");
+    for (i = 0; i < lookup->size; i++) {
+        error_append_hint(errp, "'%s'", lookup->array[i]);
+        if (i + 1 < lookup->size) {
+            error_append_hint(errp, ", ");
+        }
+    }
+    error_append_hint(errp, "\n");
+}
+
+
 static bool input_type_enum(Visitor *v, const char *name, int *obj,
                             const QEnumLookup *lookup, Error **errp)
 {
+    ERRP_GUARD();
     int64_t value;
     g_autofree char *enum_str = NULL;
 
@@ -406,6 +422,7 @@ static bool input_type_enum(Visitor *v, const char *name, int *obj,
     if (value < 0) {
         error_setg(errp, "Parameter '%s' does not accept value '%s'",
                    name ? name : "null", enum_str);
+        error_append_qapi_enum_hint(errp, lookup);
         return false;
     }
 
-- 
2.39.2

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> This allows for a more pleasant user experience.
>
> Before:
> $ ./qemu-system-x86_64 -display egl-headless,gl=
> qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value ''
>
> After:
> $ ./qemu-system-x86_64 -display egl-headless,gl=
> qemu-system-x86_64: -display egl-headless,gl=: Parameter 'gl' does not accept value ''
> Available: 'off', 'on', 'core', 'es'

I think "Acceptable values are ..." would go more nicely with "does not
accept".  What do you think?

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qapi/qapi-visit-core.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 6c13510a2b..45e39ff533 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -392,9 +392,25 @@ static bool output_type_enum(Visitor *v, const char *name, int *obj,
>      return visit_type_str(v, name, &enum_str, errp);
>  }
>  
> +static void error_append_qapi_enum_hint(Error **errp, const QEnumLookup *lookup)

error_append_hint() has Error *const *errp to make it obvious[*] that
the function does *not* set an error (by assigning to *errp).  We need
to do the same here.

See also commit 49fbc7236dd (error: make Error **errp const where it is
appropriate).

> +{
> +    int i;
> +
> +    error_append_hint(errp, "Available: ");
> +    for (i = 0; i < lookup->size; i++) {
> +        error_append_hint(errp, "'%s'", lookup->array[i]);
> +        if (i + 1 < lookup->size) {
> +            error_append_hint(errp, ", ");
> +        }
> +    }
> +    error_append_hint(errp, "\n");
> +}
> +
> +
>  static bool input_type_enum(Visitor *v, const char *name, int *obj,
>                              const QEnumLookup *lookup, Error **errp)
>  {
> +    ERRP_GUARD();
>      int64_t value;
>      g_autofree char *enum_str = NULL;
>  
> @@ -406,6 +422,7 @@ static bool input_type_enum(Visitor *v, const char *name, int *obj,
>      if (value < 0) {
>          error_setg(errp, "Parameter '%s' does not accept value '%s'",
>                     name ? name : "null", enum_str);
> +        error_append_qapi_enum_hint(errp, lookup);
>          return false;
>      }

Both of my review comments could be addressed without a respin.  Up to
you!


[*] Obvious to C language lawyers, I guess.  And also to Coccinelle
scripts.