The daemon side of this API has been broken ever since the API was introduced in 2012. Instead of sending the error from virDomainGetSecurityLabelList via RPC so that the client can see it, the dispatcher would just send a successful reply with return value set to -1 (and an empty array of labels). The client side would propagate this return value so the client can see the API failed, but the original error would be lost. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/remote/remote_daemon_dispatch.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 6c56e9ec3e..4d993afee6 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -2637,13 +2637,11 @@ remoteDispatchDomainGetSecurityLabelList(virNetServer *server G_GNUC_UNUSED, if (!(dom = get_nonnull_domain(conn, args->dom))) goto cleanup; - if ((len = virDomainGetSecurityLabelList(dom, &seclabels)) < 0) { - ret->ret = len; - ret->labels.labels_len = 0; - ret->labels.labels_val = NULL; - goto done; - } + if ((len = virDomainGetSecurityLabelList(dom, &seclabels)) < 0) + goto cleanup; + ret->ret = len; + ret->labels.labels_len = len; ret->labels.labels_val = g_new0(remote_domain_get_security_label_ret, len); for (i = 0; i < len; i++) { @@ -2653,9 +2651,7 @@ remoteDispatchDomainGetSecurityLabelList(virNetServer *server G_GNUC_UNUSED, cur->label.label_len = label_len; cur->enforcing = seclabels[i].enforcing; } - ret->labels.labels_len = ret->ret = len; - done: rv = 0; cleanup: -- 2.39.1

On a Thursday in 2023, Jiri Denemark wrote: Can you elaborate on why you find this useless? If ninfo is less than zero it will get promoted to unsigned here and the loop will take a long time, freeing some fun stuff in the meantime. Jano

In case the API returned success and a NULL pointer in uri_out, we would leak the preallocated buffer used for storing the uri_out pointer. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/remote/remote_daemon_dispatch.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index dbe6825fb8..6cf88e22c3 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3061,15 +3061,14 @@ remoteDispatchDomainMigratePrepare2(virNetServer *server G_GNUC_UNUSED, */ ret->cookie.cookie_len = cookielen; ret->cookie.cookie_val = cookie; - ret->uri_out = *uri_out == NULL ? NULL : uri_out; + ret->uri_out = *uri_out == NULL ? NULL : g_steal_pointer(&uri_out); rv = 0; cleanup: - if (rv < 0) { + if (rv < 0) virNetMessageSaveError(rerr); - VIR_FREE(uri_out); - } + VIR_FREE(uri_out); return rv; } @@ -4776,15 +4775,14 @@ remoteDispatchDomainMigratePrepare3(virNetServer *server G_GNUC_UNUSED, */ ret->cookie_out.cookie_out_len = cookieoutlen; ret->cookie_out.cookie_out_val = cookieout; - ret->uri_out = *uri_out == NULL ? NULL : uri_out; + ret->uri_out = *uri_out == NULL ? NULL : g_steal_pointer(&uri_out); rv = 0; cleanup: - if (rv < 0) { + if (rv < 0) virNetMessageSaveError(rerr); - VIR_FREE(uri_out); - } + VIR_FREE(uri_out); return rv; } @@ -5572,16 +5570,15 @@ remoteDispatchDomainMigratePrepare3Params(virNetServer *server G_GNUC_UNUSED, ret->cookie_out.cookie_out_len = cookieoutlen; ret->cookie_out.cookie_out_val = cookieout; - ret->uri_out = !*uri_out ? NULL : uri_out; + ret->uri_out = !*uri_out ? NULL : g_steal_pointer(&uri_out); rv = 0; cleanup: virTypedParamsFree(params, nparams); - if (rv < 0) { + if (rv < 0) virNetMessageSaveError(rerr); - VIR_FREE(uri_out); - } + VIR_FREE(uri_out); return rv; } -- 2.39.1