[v3] Configurable policy for handling deprecated interfaces

[PATCH v3 24/34] qapi: Replace qmp_dispatch()'s TODO comment by an explanation

Posted by Markus Armbruster 5 years, 11 months ago

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/qmp-dispatch.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 112d29a9ab..fb53687ce9 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -164,7 +164,11 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
         g_assert(!ret);
         return NULL;
     } else if (!ret) {
-        /* TODO turn into assertion */
+        /*
+         * When the command's schema has no 'returns', cmd->fn()
+         * leaves @ret null.  The QMP spec calls for an the empty
+         * object then; supply it.
+         */
         ret = QOBJECT(qdict_new());
     }
 
-- 
2.21.1

Re: [PATCH v3 24/34] qapi: Replace qmp_dispatch()'s TODO comment by an explanation

Posted by Marc-André Lureau 5 years, 10 months ago

Hi

On Sun, Mar 15, 2020 at 3:48 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/qmp-dispatch.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 112d29a9ab..fb53687ce9 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -164,7 +164,11 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
>          g_assert(!ret);
>          return NULL;
>      } else if (!ret) {
> -        /* TODO turn into assertion */
> +        /*
> +         * When the command's schema has no 'returns', cmd->fn()
> +         * leaves @ret null.  The QMP spec calls for an the empty

"for an"

Can we assert that the command's schema has no 'returns' in this case?

> +         * object then; supply it.
> +         */
>          ret = QOBJECT(qdict_new());
>      }
>
> --
> 2.21.1
>
>


-- 
Marc-André Lureau

Re: [PATCH v3 24/34] qapi: Replace qmp_dispatch()'s TODO comment by an explanation

Posted by Markus Armbruster 5 years, 10 months ago

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Sun, Mar 15, 2020 at 3:48 PM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  qapi/qmp-dispatch.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
>> index 112d29a9ab..fb53687ce9 100644
>> --- a/qapi/qmp-dispatch.c
>> +++ b/qapi/qmp-dispatch.c
>> @@ -164,7 +164,11 @@ QDict *qmp_dispatch(QmpCommandList *cmds, QObject *request,
>>          g_assert(!ret);
>>          return NULL;
>>      } else if (!ret) {
>> -        /* TODO turn into assertion */
>> +        /*
>> +         * When the command's schema has no 'returns', cmd->fn()
>> +         * leaves @ret null.  The QMP spec calls for an the empty
>
> "for an"

Fixing, thanks!

> Can we assert that the command's schema has no 'returns' in this case?

Feels impractical.  Enforcing "no returns when success-response is
false" is the QAPI schema frontend's job anyway.  That's a separate
patch.  I can put it on my to-do list.

>> +         * object then; supply it.
>> +         */
>>          ret = QOBJECT(qdict_new());
>>      }
>>
>> --
>> 2.21.1
>>
>>