Eduardo Habkost <ehabk...@redhat.com> writes:
> On Tue, Sep 22, 2020 at 05:00:41PM -0400, John Snow wrote:
>> Signed-off-by: John Snow <js...@redhat.com>
>> ---
>> scripts/qapi/events.py | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
>> index 00a9513c16..e859fd7a52 100644
>> --- a/scripts/qapi/events.py
>> +++ b/scripts/qapi/events.py
>> @@ -52,8 +52,10 @@ def gen_event_send_decl(name: str,
>> proto=build_event_send_proto(name, arg_type, boxed))
>>
>>
>> -# Declare and initialize an object 'qapi' using parameters from
>> build_params()
>> def gen_param_var(typ: QAPISchemaObjectType) -> str:
>> + """
>> + Declare and initialize a qapi object, using parameters from
>> `build_params`.
>
> The mention of "object 'qapi'" is gone, and this seems correct
> because there's no object called 'qapi' anywhere in this
> function. So:
>
> Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
>
> Comments/questions for follow up patches, because it's beyond the
> scope of this series:
>
> - Was the doc string supposed to say "an object 'param'" instead
> of "an object 'qapi'", as that's the name of the variable it
> declares?
Checking history... yes. The variable was renamed from @qapi to @param
during review.
> - The "using parameters from build_params()" part was confusing to
> me. I thought it meant gen_param_var() would call build_params().
> I took a while to notice it actually meant "using the C
> function parameters that were previously declared using
> build_params() at build_event_send_proto()". I don't know
> how we could make it clearer.
What about:
Generate a QAPI struct variable holding the event parameters,
initialized with the function's arguments.
>> + """
>> assert not typ.variants
>> ret = mcgen('''
>> %(c_name)s param = {
>> --
>> 2.26.2
>>
