diff mbox series

[v6,12/21] libqtest: add in-process qtest.c tx/rx handlers

Message ID 20191129213424.6290-13-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
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
---
 tests/libqtest.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqtest.h |  3 ++-
 2 files changed, 56 insertions(+), 1 deletion(-)

Comments

Stefan Hajnoczi Jan. 3, 2020, 11:15 a.m. UTC | #1
On Fri, Nov 29, 2019 at 09:34:47PM +0000, Oleinik, Alexander wrote:
> +QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
> +                    void (*send)(void*, const char*))
> +{
> +    QTestState *qts;
> +    qts = g_new0(QTestState, 1);
> +    *s = qts; /* Expose qts early on, since the query endianness relies on it */
> +    qts->wstatus = 0;
> +    for (int i = 0; i < MAX_IRQ; i++) {
> +        qts->irq_level[i] = false;
> +    }
> +
> +    qtest_client_set_rx_handler(qts, qtest_client_inproc_recv_line);
> +
> +    /* send() may not have a matching protoype, so use a type-safe wrapper */
> +    qts->ops.external_send = send;
> +    qtest_client_set_tx_handler(qts, send_wrapper);
> +
> +    qts->big_endian = qtest_query_target_endianness(qts);
> +    gchar *bin_path = g_strconcat("/qemu-system-", arch, NULL);
> +    setenv("QTEST_QEMU_BINARY", bin_path, 0);
> +    g_free(bin_path);

Is this a dummy path that is needed to make other code happy?  Or does
the user need to have an actual file at /qemu-system-ARCH?
Alexander Bulekov Jan. 5, 2020, 7:55 p.m. UTC | #2
On 200103 1115, Stefan Hajnoczi wrote:
> On Fri, Nov 29, 2019 at 09:34:47PM +0000, Oleinik, Alexander wrote:
> > +QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
> > +                    void (*send)(void*, const char*))
> > +{
> > +    QTestState *qts;
> > +    qts = g_new0(QTestState, 1);
> > +    *s = qts; /* Expose qts early on, since the query endianness relies on it */
> > +    qts->wstatus = 0;
> > +    for (int i = 0; i < MAX_IRQ; i++) {
> > +        qts->irq_level[i] = false;
> > +    }
> > +
> > +    qtest_client_set_rx_handler(qts, qtest_client_inproc_recv_line);
> > +
> > +    /* send() may not have a matching protoype, so use a type-safe wrapper */
> > +    qts->ops.external_send = send;
> > +    qtest_client_set_tx_handler(qts, send_wrapper);
> > +
> > +    qts->big_endian = qtest_query_target_endianness(qts);
> > +    gchar *bin_path = g_strconcat("/qemu-system-", arch, NULL);
> > +    setenv("QTEST_QEMU_BINARY", bin_path, 0);
> > +    g_free(bin_path);
> 
> Is this a dummy path that is needed to make other code happy?  Or does
> the user need to have an actual file at /qemu-system-ARCH?

Yes - with the inproc mode this is only needed to make qtest_get_arch
happy, which simply returns the suffix of the env variable. Standard
qtest initialization relies on it in qtest_init_without_qmp_handshake,
but that function is not used by the fuzzer.
Stefan Hajnoczi Jan. 8, 2020, 5:03 p.m. UTC | #3
On Sun, Jan 05, 2020 at 02:55:44PM -0500, Alexander Bulekov wrote:
> On 200103 1115, Stefan Hajnoczi wrote:
> > On Fri, Nov 29, 2019 at 09:34:47PM +0000, Oleinik, Alexander wrote:
> > > +QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
> > > +                    void (*send)(void*, const char*))
> > > +{
> > > +    QTestState *qts;
> > > +    qts = g_new0(QTestState, 1);
> > > +    *s = qts; /* Expose qts early on, since the query endianness relies on it */
> > > +    qts->wstatus = 0;
> > > +    for (int i = 0; i < MAX_IRQ; i++) {
> > > +        qts->irq_level[i] = false;
> > > +    }
> > > +
> > > +    qtest_client_set_rx_handler(qts, qtest_client_inproc_recv_line);
> > > +
> > > +    /* send() may not have a matching protoype, so use a type-safe wrapper */
> > > +    qts->ops.external_send = send;
> > > +    qtest_client_set_tx_handler(qts, send_wrapper);
> > > +
> > > +    qts->big_endian = qtest_query_target_endianness(qts);
> > > +    gchar *bin_path = g_strconcat("/qemu-system-", arch, NULL);
> > > +    setenv("QTEST_QEMU_BINARY", bin_path, 0);
> > > +    g_free(bin_path);
> > 
> > Is this a dummy path that is needed to make other code happy?  Or does
> > the user need to have an actual file at /qemu-system-ARCH?
> 
> Yes - with the inproc mode this is only needed to make qtest_get_arch
> happy, which simply returns the suffix of the env variable. Standard
> qtest initialization relies on it in qtest_init_without_qmp_handshake,
> but that function is not used by the fuzzer.

Cool, please add a comment to the code.
diff mbox series

Patch

diff --git a/tests/libqtest.c b/tests/libqtest.c
index a7df92319a..e0bc5bbe0b 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -1369,3 +1369,57 @@  static void qtest_client_set_rx_handler(QTestState *s, QTestRecvFn recv)
 {
     s->ops.recv_line = recv;
 }
+/* A type-safe wrapper for s->send() */
+static void send_wrapper(QTestState *s, const char *buf)
+{
+    s->ops.external_send(s, buf);
+}
+
+static GString *qtest_client_inproc_recv_line(QTestState *s)
+{
+    GString *line;
+    size_t offset;
+    char *eol;
+
+    eol = strchr(s->rx->str, '\n');
+    offset = eol - s->rx->str;
+    line = g_string_new_len(s->rx->str, offset);
+    g_string_erase(s->rx, 0, offset + 1);
+    return line;
+}
+
+QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
+                    void (*send)(void*, const char*))
+{
+    QTestState *qts;
+    qts = g_new0(QTestState, 1);
+    *s = qts; /* Expose qts early on, since the query endianness relies on it */
+    qts->wstatus = 0;
+    for (int i = 0; i < MAX_IRQ; i++) {
+        qts->irq_level[i] = false;
+    }
+
+    qtest_client_set_rx_handler(qts, qtest_client_inproc_recv_line);
+
+    /* send() may not have a matching protoype, so use a type-safe wrapper */
+    qts->ops.external_send = send;
+    qtest_client_set_tx_handler(qts, send_wrapper);
+
+    qts->big_endian = qtest_query_target_endianness(qts);
+    gchar *bin_path = g_strconcat("/qemu-system-", arch, NULL);
+    setenv("QTEST_QEMU_BINARY", bin_path, 0);
+    g_free(bin_path);
+
+    return qts;
+}
+
+void qtest_client_inproc_recv(void *opaque, const char *str)
+{
+    QTestState *qts = *(QTestState **)opaque;
+
+    if (!qts->rx) {
+        qts->rx = g_string_new(NULL);
+    }
+    g_string_append(qts->rx, str);
+    return;
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 0e9b8908ef..f5cf93c386 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -729,7 +729,8 @@  bool qtest_probe_child(QTestState *s);
  */
 void qtest_set_expected_status(QTestState *s, int status);
 
-QTestState *qtest_inproc_init(bool log, const char* arch,
+QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
                     void (*send)(void*, const char*));
+
 void qtest_client_inproc_recv(void *opaque, const char *str);
 #endif