On 04/01/2015 01:20 PM, Peter Krempa wrote:
> My intention is to split qemuMonitorJSONBlockJob() into simpler separate
> functions for every block job type. Since the error handling code is the
> same for all block jobs, this patch extracts the code into a separate
> function that will later be reused in more places.
> ---
> src/qemu/qemu_monitor_json.c | 64 +++++++++++++++++++++++++++-----------------
> 1 file changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index aa844ca..edd0a3f 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4252,6 +4252,39 @@ qemuMonitorJSONBlockJobInfo(qemuMonitorPtr mon,
> }
>
>
> +static int
> +qemuMonitorJSONBlockJobError(virJSONValuePtr reply,
> + const char *cmd_name,
> + const char *device)
> +{
> + if (!virJSONValueObjectHasKey(reply, "error"))
> + return 0;
> +
> + if (qemuMonitorJSONHasError(reply, "DeviceNotActive")) {
> + virReportError(VIR_ERR_OPERATION_INVALID,
> + _("No active operation on device: %s"), device);
> + } else if (qemuMonitorJSONHasError(reply, "DeviceInUse")) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("Device %s in use"), device);
> + } else if (qemuMonitorJSONHasError(reply, "NotSupported")) {
> + virReportError(VIR_ERR_OPERATION_INVALID,
> + _("Operation is not supported for device: %s"), device);
> + } else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
> + virReportError(VIR_ERR_OPERATION_INVALID,
> + _("Command '%s' is not found"), cmd_name);
> + } else {
> + virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
Because you touched it - Coverity whines that 'error' is not checked for
NULL:
(8) Event returned_null: "virJSONValueObjectGet" returns null (checked
96 out of 99 times). [details]
(16) Event var_assigned: Assigning: "error" = null return value from
"virJSONValueObjectGet".
Also see events:
(17) Event dereference: Dereferencing a pointer that might be null
"error" when calling "virJSONValueObjectGetString". [details]
There's many examples (96) of checking...
ACK with the check
John
> +
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unexpected error: (%s) '%s'"),
> + NULLSTR(virJSONValueObjectGetString(error, "class")),
> + NULLSTR(virJSONValueObjectGetString(error, "desc")));
> + }
> +
> + return -1;
> +}
> +
> +
> /* speed is in bytes/sec */
> int
> qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
> @@ -4322,34 +4355,15 @@ qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
> if (!cmd)
> return -1;
>
> - ret = qemuMonitorJSONCommand(mon, cmd, &reply);
> + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> + goto cleanup;
>
> - if (ret == 0 && virJSONValueObjectHasKey(reply, "error")) {
> - ret = -1;
> - if (qemuMonitorJSONHasError(reply, "DeviceNotActive")) {
> - virReportError(VIR_ERR_OPERATION_INVALID,
> - _("No active operation on device: %s"),
> - device);
> - } else if (qemuMonitorJSONHasError(reply, "DeviceInUse")) {
> - virReportError(VIR_ERR_OPERATION_FAILED,
> - _("Device %s in use"), device);
> - } else if (qemuMonitorJSONHasError(reply, "NotSupported")) {
> - virReportError(VIR_ERR_OPERATION_INVALID,
> - _("Operation is not supported for device: %s"),
> - device);
> - } else if (qemuMonitorJSONHasError(reply, "CommandNotFound")) {
> - virReportError(VIR_ERR_OPERATION_INVALID,
> - _("Command '%s' is not found"), cmd_name);
> - } else {
> - virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
> + if (qemuMonitorJSONBlockJobError(reply, cmd_name, device) < 0)
> + goto cleanup;
>
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Unexpected error: (%s) '%s'"),
> - NULLSTR(virJSONValueObjectGetString(error, "class")),
> - NULLSTR(virJSONValueObjectGetString(error, "desc")));
> - }
> - }
> + ret = 0;
>
> + cleanup:
> virJSONValueFree(cmd);
> virJSONValueFree(reply);
> return ret;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list