On 10/9/2023 7:43 PM, Fabiano Rosas wrote:
Het Gala<het.g...@nutanix.com> writes:
On 10/4/2023 11:42 PM, Fabiano Rosas wrote:
Daniel P. Berrangé<berra...@redhat.com> writes:
On Wed, Oct 04, 2023 at 11:43:12AM -0300, Fabiano Rosas wrote:
Het Gala<het.g...@nutanix.com> writes:
This patch parses 'migrate' and 'migrate-incoming' QAPI's 'uri'
string containing migration connection related information
and stores them inside well defined 'MigrateAddress' struct.
Suggested-by: Aravind Retnakaran<aravind.retnaka...@nutanix.com>
Signed-off-by: Het Gala<het.g...@nutanix.com>
Reviewed-by: Daniel P. Berrangé<berra...@redhat.com>
---
migration/exec.c | 1 -
migration/exec.h | 4 ++++
migration/migration.c | 55 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/migration/exec.c b/migration/exec.c
index 2bf882bbe1..32f5143dfd 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -27,7 +27,6 @@
#include "qemu/cutils.h"
#ifdef WIN32
-const char *exec_get_cmd_path(void);
const char *exec_get_cmd_path(void)
{
g_autofree char *detected_path = g_new(char, MAX_PATH);
diff --git a/migration/exec.h b/migration/exec.h
index b210ffde7a..736cd71028 100644
--- a/migration/exec.h
+++ b/migration/exec.h
@@ -19,6 +19,10 @@
#ifndef QEMU_MIGRATION_EXEC_H
rate_add_address(SocketAddress *address)
ng it out of scope at the
end of the function.
It is still good to use g_autoptr to deal with the error paths.
On the success path though you need g_steal_pointer(&addr) to
prevent the autofree cleanup running.
Ah good point, this has been suggested in an earlier version already, I
forgot to mention. We should definitely use g_steal_pointer() whenever
the variable goes out of scope.
Okay. I get the point that g_autoptr helps to deal with freeing of
pointer in case error occurs inside the function.
Yes, but note g_autoptr will free the memory any time the variable goes
out of scope, i.e. any time we return from the function. Even when
there's no error and we actually want that memory to still be around for
the callers to use.
But I am still trying to figure out why we need g_steal_pointer() ? If
you could please explain once again.
My understanding till now was that if we want to return 'addr' variable
as return type, then we would want to make use of g_steal_pointer(&addr)
so instead of addr, we pass a temp ptr refrencing to the same location
as addr, and then assign addr = NULL. But we are already assigning the
memory location where addr was refrencing to 'channel'. Let me know if I
am missing something ?
So now 'channel' points to the memory you allocated at the start of the
function with g_new0. When the function returns, g_autoptr has no idea
that someone is still using that memory, so it will just free it.
Whenever you want a chunk of memory to be accessed across function calls
like that you need to steal the reference. After stealing, the pointer
that was registered with g_autoptr (in this case 'addr') now points to
nothing and the pointer that was returned/assigned is a different one
that isn't known by any cleanup functions.
Note that after g_steal_pointer, that memory now becomes responsibility
of whatever piece of code owns that pointer. In this case,
qemu_start_incoming_migration() and qmp_migrate(). Those two functions
will have to free the memory once they are done with it. Or do the
g_autoptr/g_steal_pointer trick once more.
Got your point perfectly now. Thanks a lot. Just so that I understood
right, summarising it in short:
g_autoptr --> helps in error paths so we use it, but for non-error paths
if it goes out of scope, it will free it so to prevent that we have to
use g_steal_pointer(). Either use g_autoptr/g_steal_pointer pair or do
it simple but then will have to take care of error paths as well as
non-error paths to free the pointers.
I think the syntax follows as the second example given here:
https://urldefense.proofpoint.com/v2/url?u=https-3A__docs.gtk.org_glib_func.steal-5Fpointer.html&d=DwIFaQ&c=s883GpUCOChKOHiocYtGcg&r=-qwZZzrw4EKSsq0BK7MBd3wW1WEpXmJeng3ZUT5uBCg&m=Fn2ZhrrsSvcV_ZUX3PUnI1zMMD7JM5o1X9LbURlU1B7O06RNnzmOS3hKI4HBe6fB&s=iJFFbZnxzfqd1xu3gRVfvdjjE_X3XT3t0aRRAIeaUUc&e=
?
Yep, that's it.
Ack.
Regards,
Het Gala
