[PATCH 43/55] hyperv: use GLib auto-cleanup in hypervInvokeMethod
Laine Stump
laine at redhat.com
Fri Jan 22 08:31:02 UTC 2021
On 1/21/21 1:51 PM, Matt Coleman wrote:
> Signed-off-by: Matt Coleman <matt at datto.com>
> ---
> src/hyperv/hyperv_wmi.c | 68 ++++++++++++++++-------------------------
> 1 file changed, 27 insertions(+), 41 deletions(-)
>
> diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
> index 0a9d4bf4fd..459d207ee7 100644
> --- a/src/hyperv/hyperv_wmi.c
> +++ b/src/hyperv/hyperv_wmi.c
> @@ -744,35 +744,36 @@ hypervInvokeMethod(hypervPrivate *priv,
> WsXmlDocH *res)
> {
> g_autoptr(hypervInvokeParamsList) params = *paramsPtr;
> - int result = -1;
> size_t i = 0;
> int returnCode;
> - WsXmlDocH paramsDocRoot = NULL;
> - client_opt_t *options = NULL;
> - WsXmlDocH response = NULL;
> + g_auto(WsXmlDocH) paramsDocRoot = NULL;
> + g_autoptr(client_opt_t) options = NULL;
> + g_auto(WsXmlDocH) response = NULL;
> WsXmlNodeH methodNode = NULL;
> - char *returnValue_xpath = NULL;
> - char *jobcode_instance_xpath = NULL;
> - char *returnValue = NULL;
> - char *instanceID = NULL;
> + g_autofree char *returnValue_xpath = NULL;
> + g_autofree char *jobcode_instance_xpath = NULL;
> + g_autofree char *returnValue = NULL;
> + g_autofree char *instanceID = NULL;
> bool completed = false;
> g_auto(virBuffer) query = VIR_BUFFER_INITIALIZER;
> - Msvm_ConcreteJob *job = NULL;
> + g_autoptr(Msvm_ConcreteJob) job = NULL;
> int jobState = -1;
> hypervParamPtr p = NULL;
> int timeout = HYPERV_JOB_TIMEOUT_MS;
>
> + *paramsPtr = NULL;
> +
> if (hypervCreateInvokeXmlDoc(params, ¶msDocRoot) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Could not create XML document"));
> - goto cleanup;
> + return -1;
> }
>
> methodNode = xml_parser_get_root(paramsDocRoot);
> if (!methodNode) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Could not get root of XML document"));
> - goto cleanup;
> + return -1;
> }
>
> /* Serialize parameters */
> @@ -783,22 +784,22 @@ hypervInvokeMethod(hypervPrivate *priv,
> case HYPERV_SIMPLE_PARAM:
> if (hypervSerializeSimpleParam(p, params->resourceUri,
> &methodNode) < 0)
> - goto cleanup;
> + return -1;
> break;
> case HYPERV_EPR_PARAM:
> if (hypervSerializeEprParam(p, priv, params->resourceUri,
> &methodNode) < 0)
> - goto cleanup;
> + return -1;
> break;
> case HYPERV_EMBEDDED_PARAM:
> if (hypervSerializeEmbeddedParam(p, params->resourceUri,
> &methodNode) < 0)
> - goto cleanup;
> + return -1;
> break;
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Unknown parameter type"));
> - goto cleanup;
> + return -1;
> }
> }
>
> @@ -807,7 +808,7 @@ hypervInvokeMethod(hypervPrivate *priv,
> options = wsmc_options_init();
> if (!options) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not init options"));
> - goto cleanup;
> + return -1;
> }
> wsmc_add_selectors_from_str(options, params->selector);
>
> @@ -824,11 +825,11 @@ hypervInvokeMethod(hypervPrivate *priv,
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not get return value for %s invocation"),
> params->method);
> - goto cleanup;
> + return -1;
> }
>
> if (virStrToLong_i(returnValue, NULL, 10, &returnCode) < 0)
> - goto cleanup;
> + return -1;
>
> if (returnCode == CIM_RETURNCODE_TRANSITION_STARTED) {
> jobcode_instance_xpath = g_strdup_printf("/s:Envelope/s:Body/p:%s_OUTPUT/p:Job/a:ReferenceParameters/"
> @@ -840,7 +841,7 @@ hypervInvokeMethod(hypervPrivate *priv,
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not get instance ID for %s invocation"),
> params->method);
> - goto cleanup;
> + return -1;
> }
>
> /*
> @@ -862,7 +863,7 @@ hypervInvokeMethod(hypervPrivate *priv,
> "WHERE InstanceID = '%s'", instanceID);
>
> if (hypervGetWmiClass(Msvm_ConcreteJob, &job) < 0 || !job)
> - goto cleanup;
> + return -1;
>
> jobState = job->data->JobState;
> switch (jobState) {
> @@ -882,44 +883,29 @@ hypervInvokeMethod(hypervPrivate *priv,
> case MSVM_CONCRETEJOB_JOBSTATE_KILLED:
> case MSVM_CONCRETEJOB_JOBSTATE_EXCEPTION:
> case MSVM_CONCRETEJOB_JOBSTATE_SERVICE:
> - goto cleanup;
> + return -1;
> default:
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Unknown invocation state"));
> - goto cleanup;
> + return -1;
> }
> }
> if (!completed && timeout < 0) {
> virReportError(VIR_ERR_OPERATION_TIMEOUT,
> _("Timeout waiting for %s invocation"), params->method);
> - goto cleanup;
> + return -1;
> }
> } else if (returnCode != CIM_RETURNCODE_COMPLETED_WITH_NO_ERROR) {
> virReportError(VIR_ERR_INTERNAL_ERROR, _("Invocation of %s returned an error: %s (%d)"),
> params->method, hypervReturnCodeToString(returnCode),
> returnCode);
> - goto cleanup;
> + return -1;
> }
>
> if (res)
> - *res = response;
> + *res = g_steal_pointer(&response);
>
> - result = 0;
> -
> - cleanup:
> - if (options)
> - wsmc_options_destroy(options);
> - if (response && (!res))
> - ws_xml_destroy_doc(response);
> - if (paramsDocRoot)
> - ws_xml_destroy_doc(paramsDocRoot);
I notice that the above three cleanup functions are only called if the
pointer is != NULL. Do you know for certain that they are safe to call
with a NULL pointer? If not, then you'll need to supply a wrapper to use
as the cleanup function.
> - VIR_FREE(returnValue_xpath);
> - VIR_FREE(jobcode_instance_xpath);
> - VIR_FREE(returnValue);
> - VIR_FREE(instanceID);
> - hypervFreeObject((hypervObject *)job);
> - *paramsPtr = NULL;
> - return result;
> + return 0;
> }
>
> /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
More information about the libvir-list
mailing list