[v3,01/21] migration-test: Use g_free() instead of free()
diff mbox series

Message ID 20200123115831.36842-2-quintela@redhat.com
State New
Headers show
Series
  • Multifd Migration Compression
Related show

Commit Message

Juan Quintela Jan. 23, 2020, 11:58 a.m. UTC
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 tests/qtest/migration-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dr. David Alan Gilbert Jan. 24, 2020, 9:37 a.m. UTC | #1
* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  tests/qtest/migration-test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 26e2e77289..b6a74a05ce 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1291,7 +1291,7 @@ static void test_multifd_tcp(void)
>      wait_for_serial("dest_serial");
>      wait_for_migration_complete(from);
>      test_migrate_end(from, to, true);
> -    free(uri);
> +    g_free(uri);
>  }
>  
>  int main(int argc, char **argv)
> -- 
> 2.24.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Philippe Mathieu-Daudé Jan. 24, 2020, 9:50 a.m. UTC | #2
On 1/23/20 12:58 PM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Nothing changed since v4 (apart it is now v3),
however it misses:

Fixes: b99784ef6c3
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

See:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg672805.html
https://www.mail-archive.com/qemu-devel@nongnu.org/msg672853.html

> ---
>   tests/qtest/migration-test.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 26e2e77289..b6a74a05ce 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1291,7 +1291,7 @@ static void test_multifd_tcp(void)
>       wait_for_serial("dest_serial");
>       wait_for_migration_complete(from);
>       test_migrate_end(from, to, true);
> -    free(uri);
> +    g_free(uri);
>   }
>   
>   int main(int argc, char **argv)
>
Daniel P. Berrangé Jan. 24, 2020, 10:39 a.m. UTC | #3
On Thu, Jan 23, 2020 at 12:58:11PM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  tests/qtest/migration-test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 26e2e77289..b6a74a05ce 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1291,7 +1291,7 @@ static void test_multifd_tcp(void)
>      wait_for_serial("dest_serial");
>      wait_for_migration_complete(from);
>      test_migrate_end(from, to, true);
> -    free(uri);
> +    g_free(uri);

Not an objection to this patch, just a general FYI.

Our min glib guarantees that g_malloc/g_free are always using the
system allocator. So using free() is not a correctness problem
these days.

In general I'd suggest eliminating both free() and g_free(), and instead
annotating the variable decl for automatic free. eg

  g_autofree char *uri = NULL;


Regards,
Daniel
Juan Quintela Jan. 24, 2020, 11:08 a.m. UTC | #4
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Thu, Jan 23, 2020 at 12:58:11PM +0100, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  tests/qtest/migration-test.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
>> index 26e2e77289..b6a74a05ce 100644
>> --- a/tests/qtest/migration-test.c
>> +++ b/tests/qtest/migration-test.c
>> @@ -1291,7 +1291,7 @@ static void test_multifd_tcp(void)
>>      wait_for_serial("dest_serial");
>>      wait_for_migration_complete(from);
>>      test_migrate_end(from, to, true);
>> -    free(uri);
>> +    g_free(uri);
>
> Not an objection to this patch, just a general FYI.
>
> Our min glib guarantees that g_malloc/g_free are always using the
> system allocator. So using free() is not a correctness problem
> these days.

Ok.  But the rest of the file uses g_malloc/g_free and friends O:-)

> In general I'd suggest eliminating both free() and g_free(), and instead
> annotating the variable decl for automatic free. eg
>
>   g_autofree char *uri = NULL;

I will investigate this, thanks.

Later, Juan.

Patch
diff mbox series

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 26e2e77289..b6a74a05ce 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1291,7 +1291,7 @@  static void test_multifd_tcp(void)
     wait_for_serial("dest_serial");
     wait_for_migration_complete(from);
     test_migrate_end(from, to, true);
-    free(uri);
+    g_free(uri);
 }
 
 int main(int argc, char **argv)