diff mbox series

[v6,05/21] libqtest: Add a layer of abstraciton to send/recv

Message ID 20191129213424.6290-6-alxndr@bu.edu
State New
Headers show
Series Add virtual device fuzzing support | expand

Commit Message

Alexander Bulekov Nov. 29, 2019, 9:34 p.m. UTC 
This makes it simple to swap the transport functions for qtest commands
to and from the qtest client. For example, now it is possible to
directly pass qtest commands to a server handler that exists within the
same process, without the standard way of writing to a file descriptor.

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/libqtest.c | 50 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 9 deletions(-)

Comments

Stefan Hajnoczi Jan. 3, 2020, 11:19 a.m. UTC | #1 
On Fri, Nov 29, 2019 at 09:34:41PM +0000, Oleinik, Alexander wrote:
> This makes it simple to swap the transport functions for qtest commands
> to and from the qtest client. For example, now it is possible to
> directly pass qtest commands to a server handler that exists within the
> same process, without the standard way of writing to a file descriptor.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>  tests/libqtest.c | 50 +++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 41 insertions(+), 9 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Philippe Mathieu-Daudé Jan. 3, 2020, 4:29 p.m. UTC | #2 
Hi Aleksander,

Typo in patch subject: "abstraction"

On 11/29/19 10:34 PM, Oleinik, Alexander wrote:
> This makes it simple to swap the transport functions for qtest commands
> to and from the qtest client. For example, now it is possible to
> directly pass qtest commands to a server handler that exists within the
> same process, without the standard way of writing to a file descriptor.
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   tests/libqtest.c | 50 +++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 41 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 91e9cb220c..ac4b6ab5f0 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -35,6 +35,17 @@
>   #define SOCKET_TIMEOUT 50
>   #define SOCKET_MAX_FDS 16
>   
> +
> +typedef void (*QTestSendFn)(QTestState *s, const char *buf);
> +typedef void (*ExternalSendFn)(void *s, const char *buf);
> +typedef GString* (*QTestRecvFn)(QTestState *);
> +
> +typedef struct QTestClientTransportOps {
> +    QTestSendFn     send;

We use strlen() so we also send a send a string. Maybe rename it send_line?

Please add a comment here about what does each function handler in this 
structure.

> +    ExternalSendFn  external_send;

Is external_send used? Apparently in patch #12. Can you add it there, 
where it is used? I don't understand why we a "type-safe wrapper" is 
called 'external'.

> +    QTestRecvFn     recv_line;
> +} QTestTransportOps;
> +
>   struct QTestState
>   {
>       int fd;
> @@ -45,6 +56,7 @@ struct QTestState
>       bool big_endian;
>       bool irq_level[MAX_IRQ];
>       GString *rx;
> +    QTestTransportOps ops;
>   };
>   
>   static GHookList abrt_hooks;
> @@ -52,6 +64,14 @@ static struct sigaction sigact_old;
>   
>   static int qtest_query_target_endianness(QTestState *s);
>   
> +static void qtest_client_socket_send(QTestState*, const char *buf);
> +static void socket_send(int fd, const char *buf, size_t size);
> +
> +static GString *qtest_client_socket_recv_line(QTestState *);
> +
> +static void qtest_client_set_tx_handler(QTestState *s, QTestSendFn send);
> +static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv);
> +
>   static int init_socket(const char *socket_path)
>   {
>       struct sockaddr_un addr;
> @@ -234,6 +254,9 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>       sock = init_socket(socket_path);
>       qmpsock = init_socket(qmp_socket_path);
>   
> +    qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
> +    qtest_client_set_tx_handler(s, qtest_client_socket_send);
> +
>       qtest_add_abrt_handler(kill_qemu_hook_func, s);
>   
>       command = g_strdup_printf("exec %s "
> @@ -379,13 +402,9 @@ static void socket_send(int fd, const char *buf, size_t size)
>       }
>   }
>   
> -static void socket_sendf(int fd, const char *fmt, va_list ap)
> +static void qtest_client_socket_send(QTestState *s, const char *buf)
>   {
> -    gchar *str = g_strdup_vprintf(fmt, ap);
> -    size_t size = strlen(str);
> -
> -    socket_send(fd, str, size);
> -    g_free(str);
> +    socket_send(s->fd, buf, strlen(buf));
>   }
>   
>   static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
> @@ -393,8 +412,11 @@ static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
>       va_list ap;
>   
>       va_start(ap, fmt);
> -    socket_sendf(s->fd, fmt, ap);
> +    gchar *str = g_strdup_vprintf(fmt, ap);
>       va_end(ap);
> +
> +    s->ops.send(s, str);
> +    g_free(str);
>   }
>   
>   /* Sends a message and file descriptors to the socket.
> @@ -431,7 +453,7 @@ static void socket_send_fds(int socket_fd, int *fds, size_t fds_num,
>       g_assert_cmpint(ret, >, 0);
>   }
>   
> -static GString *qtest_recv_line(QTestState *s)
> +static GString *qtest_client_socket_recv_line(QTestState *s)
>   {
>       GString *line;
>       size_t offset;
> @@ -468,7 +490,7 @@ static gchar **qtest_rsp(QTestState *s, int expected_args)
>       int i;
>   
>   redo:
> -    line = qtest_recv_line(s);
> +    line = s->ops.recv_line(s);
>       words = g_strsplit(line->str, " ", 0);
>       g_string_free(line, TRUE);
>   
> @@ -1337,3 +1359,13 @@ void qmp_assert_error_class(QDict *rsp, const char *class)
>   
>       qobject_unref(rsp);
>   }
> +
> +static void qtest_client_set_tx_handler(QTestState *s,
> +                    QTestSendFn send)
> +{
> +    s->ops.send = send;
> +}
> +static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv)
> +{
> +    s->ops.recv_line = recv;
> +}
>
diff mbox series

Patch

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 91e9cb220c..ac4b6ab5f0 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -35,6 +35,17 @@ 
 #define SOCKET_TIMEOUT 50
 #define SOCKET_MAX_FDS 16
 
+
+typedef void (*QTestSendFn)(QTestState *s, const char *buf);
+typedef void (*ExternalSendFn)(void *s, const char *buf);
+typedef GString* (*QTestRecvFn)(QTestState *);
+
+typedef struct QTestClientTransportOps {
+    QTestSendFn     send;
+    ExternalSendFn  external_send;
+    QTestRecvFn     recv_line;
+} QTestTransportOps;
+
 struct QTestState
 {
     int fd;
@@ -45,6 +56,7 @@  struct QTestState
     bool big_endian;
     bool irq_level[MAX_IRQ];
     GString *rx;
+    QTestTransportOps ops;
 };
 
 static GHookList abrt_hooks;
@@ -52,6 +64,14 @@  static struct sigaction sigact_old;
 
 static int qtest_query_target_endianness(QTestState *s);
 
+static void qtest_client_socket_send(QTestState*, const char *buf);
+static void socket_send(int fd, const char *buf, size_t size);
+
+static GString *qtest_client_socket_recv_line(QTestState *);
+
+static void qtest_client_set_tx_handler(QTestState *s, QTestSendFn send);
+static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv);
+
 static int init_socket(const char *socket_path)
 {
     struct sockaddr_un addr;
@@ -234,6 +254,9 @@  QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
+    qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
+    qtest_client_set_tx_handler(s, qtest_client_socket_send);
+
     qtest_add_abrt_handler(kill_qemu_hook_func, s);
 
     command = g_strdup_printf("exec %s "
@@ -379,13 +402,9 @@  static void socket_send(int fd, const char *buf, size_t size)
     }
 }
 
-static void socket_sendf(int fd, const char *fmt, va_list ap)
+static void qtest_client_socket_send(QTestState *s, const char *buf)
 {
-    gchar *str = g_strdup_vprintf(fmt, ap);
-    size_t size = strlen(str);
-
-    socket_send(fd, str, size);
-    g_free(str);
+    socket_send(s->fd, buf, strlen(buf));
 }
 
 static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
@@ -393,8 +412,11 @@  static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
     va_list ap;
 
     va_start(ap, fmt);
-    socket_sendf(s->fd, fmt, ap);
+    gchar *str = g_strdup_vprintf(fmt, ap);
     va_end(ap);
+
+    s->ops.send(s, str);
+    g_free(str);
 }
 
 /* Sends a message and file descriptors to the socket.
@@ -431,7 +453,7 @@  static void socket_send_fds(int socket_fd, int *fds, size_t fds_num,
     g_assert_cmpint(ret, >, 0);
 }
 
-static GString *qtest_recv_line(QTestState *s)
+static GString *qtest_client_socket_recv_line(QTestState *s)
 {
     GString *line;
     size_t offset;
@@ -468,7 +490,7 @@  static gchar **qtest_rsp(QTestState *s, int expected_args)
     int i;
 
 redo:
-    line = qtest_recv_line(s);
+    line = s->ops.recv_line(s);
     words = g_strsplit(line->str, " ", 0);
     g_string_free(line, TRUE);
 
@@ -1337,3 +1359,13 @@  void qmp_assert_error_class(QDict *rsp, const char *class)
 
     qobject_unref(rsp);
 }
+
+static void qtest_client_set_tx_handler(QTestState *s,
+                    QTestSendFn send)
+{
+    s->ops.send = send;
+}
+static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv)
+{
+    s->ops.recv_line = recv;
+}