On 26.05.2016 13:14, Maxim Nestratov wrote:
> If PrlEvent_GetErrCode returns an error code this is what we should
> use as an error code for the whole action
>
> Signed-off-by: Maxim Nestratov <mnestratov@xxxxxxxxxxxxx>
> ---
> src/vz/vz_sdk.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index e5c56a5..7fc7d97 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -100,14 +100,17 @@ logPrlErrorHelper(PRL_RESULT err, const char *filename,
>
> static PRL_RESULT
> logPrlEventErrorHelper(PRL_HANDLE event, const char *filename,
> - const char *funcname, size_t linenr)
> + const char *funcname, size_t linenr, PRL_RESULT* retCode)
I would put retCode before caller position parameters(filename etc)
> {
> - PRL_RESULT ret, retCode;
> + PRL_RESULT ret;
> char *msg1 = NULL, *msg2 = NULL;
> PRL_UINT32 len = 0;
> int err = -1;
>
> - if ((ret = PrlEvent_GetErrCode(event, &retCode))) {
> + if (!retCode)
> + return -1;
Supplying NULL retCode is a big usage mistake as you get wrong result code in this
case. Thus I would instead don't fail but give warning and continue. This way
correct error will be logged at least.
Eventually I think simpliest would be move PrlEvent_GetErrCode out of this function.
Also I would make it void as it is a logging function.
> +
> + if ((ret = PrlEvent_GetErrCode(event, retCode))) {
> logPrlError(ret);
> return ret;
> }
> @@ -164,7 +167,8 @@ getJobResultHelper(PRL_HANDLE job, unsigned int timeout, PRL_HANDLE *result,
> goto cleanup;
> }
>
> - if (logPrlEventErrorHelper(err_handle, filename, funcname, linenr))
> + ret = logPrlEventErrorHelper(err_handle, filename, funcname, linenr, &retCode);
> + if (ret)
> logPrlErrorHelper(retCode, filename, funcname, linenr);
>
> PrlHandle_Free(err_handle);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list