[Qemu-devel] [PATCH 0/2] libqtest leak fix & cleanup

[Qemu-devel] [PATCH 0/2] libqtest leak fix & cleanup

[Markus Armbruster]

Markus Armbruster (2):
  libqtest: Plug fd and memory leaks in qtest_quit()
  libqtest: New qtest_end() to go with qtest_start()

 tests/fdc-test.c    | 2 +-
 tests/hd-geo-test.c | 8 ++++----
 tests/ide-test.c    | 2 +-
 tests/libqtest.c    | 4 ++++
 tests/libqtest.h    | 7 +++++++
 5 files changed, 17 insertions(+), 6 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH 1/2] libqtest: Plug fd and memory leaks in qtest_quit()

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/libqtest.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 879ffe9..bb82069 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -171,12 +171,16 @@ void qtest_quit(QTestState *s)
         waitpid(pid, &status, 0);
     }
 
+    close(s->fd);
+    close(s->qmp_fd);
+    g_string_free(s->rx, true);
     unlink(s->pid_file);
     unlink(s->socket_path);
     unlink(s->qmp_socket_path);
     g_free(s->pid_file);
     g_free(s->socket_path);
     g_free(s->qmp_socket_path);
+    g_free(s);
 }
 
 static void socket_sendf(int fd, const char *fmt, va_list ap)
-- 
1.7.11.7

[Qemu-devel] [PATCH 2/2] libqtest: New qtest_end() to go with qtest_start()

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/fdc-test.c    | 2 +-
 tests/hd-geo-test.c | 8 ++++----
 tests/ide-test.c    | 2 +-
 tests/libqtest.h    | 7 +++++++
 4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 4b0301d..fd198dc 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -556,7 +556,7 @@ int main(int argc, char **argv)
     ret = g_test_run();
 
     /* Cleanup */
-    qtest_quit(global_qtest);
+    qtest_end();
     unlink(test_image);
 
     return ret;
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 9a31e85..b72042e 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -244,7 +244,7 @@ static void test_ide_none(void)
     setup_common(argv, ARRAY_SIZE(argv));
     qtest_start(g_strjoinv(" ", argv));
     test_cmos();
-    qtest_quit(global_qtest);
+    qtest_end();
 }
 
 static void test_ide_mbr(bool use_device, MBRcontents mbr)
@@ -262,7 +262,7 @@ static void test_ide_mbr(bool use_device, MBRcontents mbr)
     }
     qtest_start(g_strjoinv(" ", argv));
     test_cmos();
-    qtest_quit(global_qtest);
+    qtest_end();
 }
 
 /*
@@ -334,7 +334,7 @@ static void test_ide_drive_user(const char *dev, bool trans)
     g_free(opts);
     qtest_start(g_strjoinv(" ", argv));
     test_cmos();
-    qtest_quit(global_qtest);
+    qtest_end();
 }
 
 /*
@@ -387,7 +387,7 @@ static void test_ide_drive_cd_0(void)
     }
     qtest_start(g_strjoinv(" ", argv));
     test_cmos();
-    qtest_quit(global_qtest);
+    qtest_end();
 }
 
 int main(int argc, char **argv)
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 7e2eb94..7307f1d 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -122,7 +122,7 @@ static void ide_test_start(const char *cmdline_fmt, ...)
 
 static void ide_test_quit(void)
 {
-    qtest_quit(global_qtest);
+    qtest_end();
 }
 
 static QPCIDevice *get_pci_device(uint16_t *bmdma_base)
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 437bda3..329325b 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -17,6 +17,7 @@
 #ifndef LIBQTEST_H
 #define LIBQTEST_H
 
+#include <stddef.h>
 #include <stdint.h>
 #include <stdbool.h>
 #include <stdarg.h>
@@ -318,6 +319,12 @@ static inline QTestState *qtest_start(const char *args)
     return global_qtest;
 }
 
+static inline void qtest_end(void)
+{
+    qtest_quit(global_qtest);
+    global_qtest = NULL;
+}
+
 /**
  * qmp:
  * @fmt...: QMP message to send to qemu
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH 1/2] libqtest: Plug fd and memory leaks in qtest_quit()

[Anthony Liguori]

Markus Armbruster <armbru@redhat.com> writes:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>  tests/libqtest.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 879ffe9..bb82069 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -171,12 +171,16 @@ void qtest_quit(QTestState *s)
>          waitpid(pid, &status, 0);
>      }
>  
> +    close(s->fd);
> +    close(s->qmp_fd);
> +    g_string_free(s->rx, true);
>      unlink(s->pid_file);
>      unlink(s->socket_path);
>      unlink(s->qmp_socket_path);
>      g_free(s->pid_file);
>      g_free(s->socket_path);
>      g_free(s->qmp_socket_path);
> +    g_free(s);
>  }
>  
>  static void socket_sendf(int fd, const char *fmt, va_list ap)
> -- 
> 1.7.11.7

Re: [Qemu-devel] [PATCH 1/2] libqtest: Plug fd and memory leaks in qtest_quit()

[Anthony Liguori]

Markus Armbruster <armbru@redhat.com> writes:

> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> ---
>  tests/libqtest.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 879ffe9..bb82069 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -171,12 +171,16 @@ void qtest_quit(QTestState *s)
>          waitpid(pid, &status, 0);
>      }
>  
> +    close(s->fd);
> +    close(s->qmp_fd);
> +    g_string_free(s->rx, true);
>      unlink(s->pid_file);
>      unlink(s->socket_path);
>      unlink(s->qmp_socket_path);
>      g_free(s->pid_file);
>      g_free(s->socket_path);
>      g_free(s->qmp_socket_path);
> +    g_free(s);
>  }
>  
>  static void socket_sendf(int fd, const char *fmt, va_list ap)
> -- 
> 1.7.11.7

Re: [Qemu-devel] [PATCH 2/2] libqtest: New qtest_end() to go with qtest_start()

[Andreas Färber]

Am 19.06.2013 09:44, schrieb Markus Armbruster:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/fdc-test.c    | 2 +-
>  tests/hd-geo-test.c | 8 ++++----
>  tests/ide-test.c    | 2 +-
>  tests/libqtest.h    | 7 +++++++
>  4 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/fdc-test.c b/tests/fdc-test.c
> index 4b0301d..fd198dc 100644
> --- a/tests/fdc-test.c
> +++ b/tests/fdc-test.c
> @@ -556,7 +556,7 @@ int main(int argc, char **argv)
>      ret = g_test_run();
>  
>      /* Cleanup */
> -    qtest_quit(global_qtest);
> +    qtest_end();
>      unlink(test_image);
>  
>      return ret;
> diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
> index 9a31e85..b72042e 100644
> --- a/tests/hd-geo-test.c
> +++ b/tests/hd-geo-test.c
> @@ -244,7 +244,7 @@ static void test_ide_none(void)
>      setup_common(argv, ARRAY_SIZE(argv));
>      qtest_start(g_strjoinv(" ", argv));
>      test_cmos();
> -    qtest_quit(global_qtest);
> +    qtest_end();
>  }
>  
>  static void test_ide_mbr(bool use_device, MBRcontents mbr)
> @@ -262,7 +262,7 @@ static void test_ide_mbr(bool use_device, MBRcontents mbr)
>      }
>      qtest_start(g_strjoinv(" ", argv));
>      test_cmos();
> -    qtest_quit(global_qtest);
> +    qtest_end();
>  }
>  
>  /*
> @@ -334,7 +334,7 @@ static void test_ide_drive_user(const char *dev, bool trans)
>      g_free(opts);
>      qtest_start(g_strjoinv(" ", argv));
>      test_cmos();
> -    qtest_quit(global_qtest);
> +    qtest_end();
>  }
>  
>  /*
> @@ -387,7 +387,7 @@ static void test_ide_drive_cd_0(void)
>      }
>      qtest_start(g_strjoinv(" ", argv));
>      test_cmos();
> -    qtest_quit(global_qtest);
> +    qtest_end();
>  }
>  
>  int main(int argc, char **argv)
> diff --git a/tests/ide-test.c b/tests/ide-test.c
> index 7e2eb94..7307f1d 100644
> --- a/tests/ide-test.c
> +++ b/tests/ide-test.c
> @@ -122,7 +122,7 @@ static void ide_test_start(const char *cmdline_fmt, ...)
>  
>  static void ide_test_quit(void)
>  {
> -    qtest_quit(global_qtest);
> +    qtest_end();
>  }
>  
>  static QPCIDevice *get_pci_device(uint16_t *bmdma_base)
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 437bda3..329325b 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -17,6 +17,7 @@
>  #ifndef LIBQTEST_H
>  #define LIBQTEST_H
>  
> +#include <stddef.h>
>  #include <stdint.h>
>  #include <stdbool.h>
>  #include <stdarg.h>
> @@ -318,6 +319,12 @@ static inline QTestState *qtest_start(const char *args)
>      return global_qtest;
>  }
>  
> +static inline void qtest_end(void)
> +{
> +    qtest_quit(global_qtest);
> +    global_qtest = NULL;
> +}
> +
>  /**
>   * qmp:
>   * @fmt...: QMP message to send to qemu
> 

Function and usage looks fine, but please add a gtk-doc comment.
qtest_start() does have one.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 2/2] libqtest: New qtest_end() to go with qtest_start()

[Markus Armbruster]

Andreas Färber <afaerber@suse.de> writes:

> Am 19.06.2013 09:44, schrieb Markus Armbruster:
[...]
>> diff --git a/tests/libqtest.h b/tests/libqtest.h
>> index 437bda3..329325b 100644
>> --- a/tests/libqtest.h
>> +++ b/tests/libqtest.h
>> @@ -17,6 +17,7 @@
>>  #ifndef LIBQTEST_H
>>  #define LIBQTEST_H
>>  
>> +#include <stddef.h>
>>  #include <stdint.h>
>>  #include <stdbool.h>
>>  #include <stdarg.h>
>> @@ -318,6 +319,12 @@ static inline QTestState *qtest_start(const char *args)
>>      return global_qtest;
>>  }
>>  
>> +static inline void qtest_end(void)
>> +{
>> +    qtest_quit(global_qtest);
>> +    global_qtest = NULL;
>> +}
>> +
>>  /**
>>   * qmp:
>>   * @fmt...: QMP message to send to qemu
>> 
>
> Function and usage looks fine, but please add a gtk-doc comment.
> qtest_start() does have one.

Oversight, will fix, thanks!