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

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> On Sun, Mar 15, 2020 at 3:48 PM Markus Armbruster  wrote:
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>>  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
>>
>>

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

[Marc-André Lureau]

Hi

On Sun, Mar 15, 2020 at 3:48 PM Markus Armbruster  wrote:
>
> Signed-off-by: Markus Armbruster 
> ---
>  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

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

[Markus Armbruster]

Signed-off-by: Markus Armbruster 
---
 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