...

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 990bfce..c62e3d6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2849,8 +2849,12 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn, goto done;

if (ret.cookie.cookie_len > 0) { - *cookie = ret.cookie.cookie_val; /* Caller frees. */ - *cookielen = ret.cookie.cookie_len; + if (cookie && cookielen) { + *cookie = ret.cookie.cookie_val; /* Caller frees. */ + *cookielen = ret.cookie.cookie_len; + } else { + VIR_FREE(ret.cookie.cookie_val); + } }

What code would call this with cookie == NULL ? Any such caller is a bug I believe.

Yeah, most likely but we shouldn't crash in that case anyway. Although perhaps the call should rather fail instead of just silently dropping the cookie if the caller is not interested in seeing it, what do you think? Jirka

On 05/13/2010 02:05 AM, Jiri Denemark wrote:

--- src/remote/remote_driver.c | 19 ++++++++++++++++++- 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 990bfce..80977a3 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2849,17 +2849,34 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn, goto done;

if (ret.cookie.cookie_len > 0) { + if (!cookie || !cookielen) { + remoteError(VIR_ERR_INTERNAL_ERROR, "%s", + _("caller ignores cookie or cookielen"));

For a moment, I wondered if VIR_ERR_INVALID_ARG would be any better here, but decided you probably did the right thing since this is implementation rather than a direct public API.

+ goto error; + } *cookie = ret.cookie.cookie_val; /* Caller frees. */ *cookielen = ret.cookie.cookie_len; } - if (ret.uri_out) + if (ret.uri_out) { + if (!uri_out) { + remoteError(VIR_ERR_INTERNAL_ERROR, "%s", + _("caller ignores uri_out")); + goto error; + } *uri_out = *ret.uri_out; /* Caller frees. */ + }

rv = 0;

done: remoteDriverUnlock(priv); return rv; +error: + if (ret.cookie.cookie_len) + VIR_FREE(ret.cookie.cookie_val); + if (ret.uri_out) + VIR_FREE(*ret.uri_out); + goto done; }

ACK; looks nicer than v1 for diagnosing a logic bug in the caller, rather than papering over it. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org