Re: [libvirt] [PATCH] Merge all returns paths from dispatcher into single path

>> + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); > > Is virNetError really the best name for the new helper macro, especially > since VIR_FROM_THIS is VIR_FROM_REMOTE? Should it instead be named > virRemoteError()? virNetError() is what my RPC series of patches uses throughout. Also it will change VIR_FROM_REMOTE to VIR_FROM_RPC

>> +cleanup: >> + if (rv < 0) >> + remoteDispatchError(rerr); > > That works. I wonder if we can completely get rid of > remoteDispatchOOMError? Other files still use it, but it will be gone with the RPC rewrite.

>> if (virDomainBlockPeek(dom, path, offset, size, >> ret->buffer.buffer_val, flags) == -1) { >> - /* free(ret->buffer.buffer_val); - caller frees */ >> - remoteDispatchConnError(rerr, conn); >> - virDomainFree(dom); >> - return -1; >> + goto cleanup; >> } >> - virDomainFree(dom); >> >> - return 0; >> + rv = 0; >> + >> +cleanup: >> + if (rv < 0) { >> + remoteDispatchError(rerr); >> + VIR_FREE(ret->buffer.buffer_val); > > Can we also clean up the caller to quit freeing (as a separate patch), > now that we guarantee the cleanup here? Should we have a 'make > syntax-check' rule that catches unadorned use of free() instead of VIR_FREE? I'm not sure what you mean here ? If rv == 0, then 'ret' is expected to have the data present, and the caller will use xdr_free() to release it. If rv == -1, then 'ret' is expected to have not been initialized, hence this code must free that buffer.