Re: [libvirt] [PATCH 2/4] vz: return event error code in logPrlEventErrorHelper and use it

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(a)virtuozzo.com&gt; > --- > 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.