On 2023/11/16 22:19, Juan Quintela wrote:
Zongmin Zhou <zhouzong...@kylinos.cn> wrote:
Since socket_parse() will allocate memory for 'saddr',and its value
will pass to 'addr' that allocated by migrate_uri_parse(),
then 'saddr' will no longer used,need to free.
But due to 'saddr->u' is shallow copying the contents of the union,
the members of this union containing allocated strings,and will be used after
that.
So just free 'saddr' itself without doing a deep free on the contents of the
SocketAddress.
Fixes: 72a8192e225c ("migration: convert migration 'uri' into 'MigrateAddress'")
Signed-off-by: Zongmin Zhou<zhouzong...@kylinos.cn>
---
migration/migration.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/migration/migration.c b/migration/migration.c
index 28a34c9068..9bdbcdaf49 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -493,6 +493,7 @@ bool migrate_uri_parse(const char *uri, MigrationChannel
**channel,
}
addr->u.socket.type = saddr->type;
addr->u.socket.u = saddr->u;
+ g_free(saddr);
} else if (strstart(uri, "file:", NULL)) {
addr->transport = MIGRATION_ADDRESS_TYPE_FILE;
addr->u.file.filename = g_strdup(uri + strlen("file:"));
Once that we are here, can we move the declaration of saddr to this
block, so we are sure that we don't use saddr anywhere?
do you mean to do the changes below at this block?
SocketAddress *saddr = socket_parse(uri, errp);
That sounds good and make it clear that 'saddr' is only used on this block.
As Peter said, putting a comment why we don't use
qapi_free_SocketAddress() will be a good idea.
I have put some comments on patch v2 to explain
why just free 'saddr' itself without doing a deep free on the contents
of the SocketAddress .
Maybe need to explicit clarify why g_free is used instead of
qapi_free_SocketAddress()?
Best regards!
Later, Juan.
