On 11.06.2016 20:16, Maxim Nestratov wrote:
> Function logPrlEventErrorHelper is made void and now it only prints
> an information (if any) from event.
> If there is no addition information (i.e. an error returned when
> asked), we don't rewrite original error code with secondary one.
I would mention the original motivation of the patch, that is returning
error code of PrlEvent_GetErrCode. And motivation of checking for
PRL_ERR_NO_DATA too.
>
> Signed-off-by: Maxim Nestratov <mnestratov@xxxxxxxxxxxxx>
> ---
> src/vz/vz_sdk.c | 33 ++++++++++++++++-----------------
> 1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index 99c5d4a..54c56e9 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -99,19 +99,13 @@ logPrlErrorHelper(PRL_RESULT err, const char *filename,
> } \
> } while (0)
>
> -static PRL_RESULT
> +static void
> logPrlEventErrorHelper(PRL_HANDLE event, const char *filename,
> const char *funcname, size_t linenr)
> {
> - PRL_RESULT ret, retCode;
> char *msg1 = NULL, *msg2 = NULL;
> PRL_UINT32 len = 0;
> - int err = -1;
>
> - if ((ret = PrlEvent_GetErrCode(event, &retCode))) {
> - logPrlError(ret);
> - return ret;
> - }
>
> PrlEvent_GetErrString(event, PRL_TRUE, PRL_FALSE, NULL, &len);
>
> @@ -130,13 +124,9 @@ logPrlEventErrorHelper(PRL_HANDLE event, const char *filename,
> virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INTERNAL_ERROR,
> filename, funcname, linenr,
> _("%s %s"), msg1, msg2);
> - err = 0;
> -
> cleanup:
> VIR_FREE(msg1);
> VIR_FREE(msg2);
> -
> - return err;
> }
>
> static PRL_RESULT
> @@ -146,12 +136,12 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result,
> {
> PRL_RESULT ret, retCode;
>
> - if ((ret = PrlJob_Wait(job, timeout))) {
> + if (PRL_FAILED(ret = PrlJob_Wait(job, timeout))) {
> logPrlErrorHelper(ret, filename, funcname, linenr);
> goto cleanup;
> }
>
> - if ((ret = PrlJob_GetRetCode(job, &retCode))) {
> + if (PRL_FAILED(ret = PrlJob_GetRetCode(job, &retCode))) {
> logPrlErrorHelper(ret, filename, funcname, linenr);
> goto cleanup;
> }
> @@ -159,17 +149,26 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result,
> if (retCode) {
> PRL_HANDLE err_handle;
>
> + ret = retCode;
> +
> /* Sometimes it's possible to get additional error info. */
> - if ((ret = PrlJob_GetError(job, &err_handle))) {
> + if (PRL_FAILED(retCode = PrlJob_GetError(job, &err_handle))) {
> logPrlErrorHelper(ret, filename, funcname, linenr);
> + if (PRL_ERR_NO_DATA != retCode)
> + logPrlError(retCode);
> goto cleanup;
> }
>
> - if (logPrlEventErrorHelper(err_handle, filename, funcname, linenr))
> - logPrlErrorHelper(retCode, filename, funcname, linenr);
> + if (PRL_FAILED(retCode = PrlEvent_GetErrCode(err_handle, &retCode))) {
> + logPrlErrorHelper(ret, filename, funcname, linenr);
> + if (PRL_ERR_NO_DATA != retCode)
looks like this error code does not matter in case of PrlEvent_GetErrCode
> + logPrlError(retCode);
> + goto cleanup;
err_handle leak
> + }
> +
> + logPrlEventErrorHelper(err_handle, filename, funcname, linenr);
>
> PrlHandle_Free(err_handle);
> - ret = retCode;
retCode of successful PrlEvent_GetErrCode is lost
> } else {
> ret = PrlJob_GetResult(job, result);
> if (PRL_FAILED(ret)) {
>
Even after this patch getJobResultHelper will not be good. If some error occurs we have no simple
way to find what function fails:
PrlJob_Wait
PrlJob_GetRetCode
PrlJob_GetError
PrlEvent_GetErrCode
PrlJob_GetResult
as they all will print: logPrlErrorHelper(ret, filename, funcname, linenr) with funcname and
linenr of the caller of getJobResultHelper.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list