On 3/19/24 15:58, Jiri Denemark wrote:
> On Tue, Mar 19, 2024 at 14:34:08 +0100, Claudio Fontana wrote:
>> On 3/19/24 12:02, Adam Julis wrote:
>>> vshAdmCatchDisconnect requires non-NULL structure vshControl for
>>> getting connection name (stored at opaque), but
>>> virAdmConnectRegisterCloseCallback at vshAdmConnect called it
>>> with NULL.
>>>
>>> Signed-off-by: Adam Julis <ajulis@xxxxxxxxxx> ---
>>> tools/virt-admin.c | 2 +- 1 file changed, 1 insertion(+), 1
>>> deletion(-)
>>>
>>> diff --git a/tools/virt-admin.c b/tools/virt-admin.c index
>>> 37bc6fe4f0..0766032e4a 100644 --- a/tools/virt-admin.c +++
>>> b/tools/virt-admin.c @@ -112,7 +112,7 @@ vshAdmConnect(vshControl
>>> *ctl, unsigned int flags) return -1; } else { if
>>> (virAdmConnectRegisterCloseCallback(priv->conn,
>>> vshAdmCatchDisconnect, -
>>> NULL, NULL) < 0) +
>>> ctl, NULL) < 0) vshError(ctl, "%s", _("Unable to register
>>> disconnect callback"));
>>>
>>> if (priv->wantReconnect)
>>
>> Hi,
>>
>> if that is the case I would then expect that
>> virAdmConnectRegisterCloseCallback() needs to also be updated
>> with:
>>
>> +virCheckNonNullArgGoto(opaque, error);
>>
>> or something like that. Is there a reason why it isn't? We better
>> catch the error early if the API is used wrongly.
>
> Well, vshAdmCatchDisconnect requires opaque to be non-NULL, but
> other callbacks registered with virAdmConnectRegisterCloseCallback
> may not need any opaque data.
>
Fair enough.
Reviewed-by: Claudio Fontana <cfontana@xxxxxxx>
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
