Re: [Qemu-devel] [RFC 07/19] fuzz: Modify libqtest to directly invoke qtest.c
On 26/07/19 14:56, Stefan Hajnoczi wrote: > This should use indirection: a function pointer to dispatch to either > the socket or the internal qtest_process_inbuf() call. > > With a bit of refactoring you can eliminate the #ifdefs and treat the > socket fd as one backend and direct invocation as another backend. My suggestion was a bit different (two files), but this also works. In fact it can also be combined to have three files: - one defining libqtest's qtest_init and associated struct of function pointers - one defining the fuzzer's qtest_init and associated struct of function pointers - one with the remaining libqtest code, modified to use the struct of function pointers for everything that you're #ifdef-ing here, and a function qtest_client_init that receives the struct of function pointers and stores them in QTestState. The two qtest_init implementations in the other files just call qtest_client_init. Paolo
Re: [Qemu-devel] [RFC 07/19] fuzz: Modify libqtest to directly invoke qtest.c
On Thu, Jul 25, 2019 at 03:23:49AM +, Oleinik, Alexander wrote:
> @@ -379,9 +399,18 @@ static void socket_sendf(int fd, const char *fmt,
> va_list ap)
> {
> gchar *str = g_strdup_vprintf(fmt, ap);
> size_t size = strlen(str);
> +#ifdef CONFIG_FUZZ
> +// Directly call qtest_process_inbuf in the qtest server
> +GString *gstr = g_string_new_len(str, size);
> + /* printf(">>> %s",gstr->str); */
> +qtest_server_recv(gstr);
> +g_string_free(gstr, true);
> +g_free(str);
> +#else
>
> socket_send(fd, str, size);
> g_free(str);
> +#endif
> }
This should use indirection: a function pointer to dispatch to either
the socket or the internal qtest_process_inbuf() call.
With a bit of refactoring you can eliminate the #ifdefs and treat the
socket fd as one backend and direct invocation as another backend.
signature.asc
Description: PGP signature
Re: [Qemu-devel] [RFC 07/19] fuzz: Modify libqtest to directly invoke qtest.c
On Thu, Jul 25, 2019 at 11:04:11AM +0200, Thomas Huth wrote:
> On 25/07/2019 05.23, Oleinik, Alexander wrote:
> > @@ -379,9 +399,18 @@ static void socket_sendf(int fd, const char *fmt,
> > va_list ap)
> > {
> > gchar *str = g_strdup_vprintf(fmt, ap);
> > size_t size = strlen(str);
> > +#ifdef CONFIG_FUZZ
> > +// Directly call qtest_process_inbuf in the qtest server
> > +GString *gstr = g_string_new_len(str, size);
> > + /* printf(">>> %s",gstr->str); */
>
> Please check your patches with scripts/checkpatch.pl - e.g. don't use
> TABs for indentation like in the above line, don't use //-comments, etc.
You can set up a git-hook with checkpatch.pl to scan code automatically
before each commit:
http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
Stefan
signature.asc
Description: PGP signature
Re: [Qemu-devel] [RFC 07/19] fuzz: Modify libqtest to directly invoke qtest.c
On 25/07/19 11:04, Thomas Huth wrote:
>> @@ -797,6 +832,9 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
>>
>> const char *qtest_get_arch(void)
>> {
>> +#ifdef CONFIG_FUZZ
>> +return "i386";
>> +#endif
>
> Hard-coding "i386" is quite ugly ... it's ok for an RFC patch, but I
> think this should be fixed in the final version of the patches. Maybe
> you could use TARGET_NAME instead?
Yes, TARGET_NAME is the one. Also I would just split the file in two:
the common bits that are used for both libqtest and fuzz in one file, so
the libqtest and fuzz "drivers" can be in completely separate file
without #ifdefs.
Paolo
>
>> const char *qemu = qtest_qemu_binary();
>> const char *end = strrchr(qemu, '/');
>>
>> @@ -1339,3 +1377,16 @@ void qmp_assert_error_class(QDict *rsp, const char
>> *class)
>>
>> qobject_unref(rsp);
>> }
>> +#ifdef CONFIG_FUZZ
>> +void qtest_clear_rxbuf(QTestState *s){
>
> For functions, the curly brace should start on a new line.
>
>> +g_string_set_size(recv_str,0);
>> +}
>
> Thomas
>
Re: [Qemu-devel] [RFC 07/19] fuzz: Modify libqtest to directly invoke qtest.c
On 25/07/2019 05.23, Oleinik, Alexander wrote:
> libqtest directly invokes the qtest client and exposes a function to
> accept responses.
>
> Signed-off-by: Alexander Oleinik
> ---
> tests/libqtest.c | 53 +++-
> tests/libqtest.h | 6 ++
> 2 files changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 3c5c3f49d8..a68a7287cb 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -30,12 +30,18 @@
> #include "qapi/qmp/qjson.h"
> #include "qapi/qmp/qlist.h"
> #include "qapi/qmp/qstring.h"
> +#ifdef CONFIG_FUZZ
> +#include "sysemu/qtest.h"
> +#endif
>
> #define MAX_IRQ 256
> #define SOCKET_TIMEOUT 50
> #define SOCKET_MAX_FDS 16
>
> QTestState *global_qtest;
> +#ifdef CONFIG_FUZZ
> +static GString *recv_str;
> +#endif
>
> struct QTestState
> {
> @@ -316,6 +322,20 @@ QTestState *qtest_initf(const char *fmt, ...)
> va_end(ap);
> return s;
> }
> +#ifdef CONFIG_FUZZ
> +QTestState *qtest_init_fuzz(const char *extra_args, int *sock_fd)
> +{
> +QTestState *qts;
> +qts = g_new(QTestState, 1);
> +qts->wstatus = 0;
> +for (int i = 0; i < MAX_IRQ; i++) {
> +qts->irq_level[i] = false;
> +}
> +qts->big_endian = qtest_query_target_endianness(qts);
> +
> +return qts;
> +}
> +#endif
>
> QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
> {
> @@ -379,9 +399,18 @@ static void socket_sendf(int fd, const char *fmt,
> va_list ap)
> {
> gchar *str = g_strdup_vprintf(fmt, ap);
> size_t size = strlen(str);
> +#ifdef CONFIG_FUZZ
> +// Directly call qtest_process_inbuf in the qtest server
> +GString *gstr = g_string_new_len(str, size);
> + /* printf(">>> %s",gstr->str); */
Please check your patches with scripts/checkpatch.pl - e.g. don't use
TABs for indentation like in the above line, don't use //-comments, etc.
> +qtest_server_recv(gstr);
> +g_string_free(gstr, true);
> +g_free(str);
> +#else
>
> socket_send(fd, str, size);
> g_free(str);
> +#endif
> }
>
> static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt,
> ...)
> @@ -433,6 +462,12 @@ static GString *qtest_recv_line(QTestState *s)
> size_t offset;
> char *eol;
>
> +#ifdef CONFIG_FUZZ
> +eol = strchr(recv_str->str, '\n');
> +offset = eol - recv_str->str;
> +line = g_string_new_len(recv_str->str, offset);
> +g_string_erase(recv_str, 0, offset + 1);
> +#else
> while ((eol = strchr(s->rx->str, '\n')) == NULL) {
> ssize_t len;
> char buffer[1024];
> @@ -453,7 +488,7 @@ static GString *qtest_recv_line(QTestState *s)
> offset = eol - s->rx->str;
> line = g_string_new_len(s->rx->str, offset);
> g_string_erase(s->rx, 0, offset + 1);
> -
> +#endif
> return line;
> }
>
> @@ -797,6 +832,9 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
>
> const char *qtest_get_arch(void)
> {
> +#ifdef CONFIG_FUZZ
> +return "i386";
> +#endif
Hard-coding "i386" is quite ugly ... it's ok for an RFC patch, but I
think this should be fixed in the final version of the patches. Maybe
you could use TARGET_NAME instead?
> const char *qemu = qtest_qemu_binary();
> const char *end = strrchr(qemu, '/');
>
> @@ -1339,3 +1377,16 @@ void qmp_assert_error_class(QDict *rsp, const char
> *class)
>
> qobject_unref(rsp);
> }
> +#ifdef CONFIG_FUZZ
> +void qtest_clear_rxbuf(QTestState *s){
For functions, the curly brace should start on a new line.
> +g_string_set_size(recv_str,0);
> +}
Thomas
[Qemu-devel] [RFC 07/19] fuzz: Modify libqtest to directly invoke qtest.c
libqtest directly invokes the qtest client and exposes a function to
accept responses.
Signed-off-by: Alexander Oleinik
---
tests/libqtest.c | 53 +++-
tests/libqtest.h | 6 ++
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 3c5c3f49d8..a68a7287cb 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -30,12 +30,18 @@
#include "qapi/qmp/qjson.h"
#include "qapi/qmp/qlist.h"
#include "qapi/qmp/qstring.h"
+#ifdef CONFIG_FUZZ
+#include "sysemu/qtest.h"
+#endif
#define MAX_IRQ 256
#define SOCKET_TIMEOUT 50
#define SOCKET_MAX_FDS 16
QTestState *global_qtest;
+#ifdef CONFIG_FUZZ
+static GString *recv_str;
+#endif
struct QTestState
{
@@ -316,6 +322,20 @@ QTestState *qtest_initf(const char *fmt, ...)
va_end(ap);
return s;
}
+#ifdef CONFIG_FUZZ
+QTestState *qtest_init_fuzz(const char *extra_args, int *sock_fd)
+{
+QTestState *qts;
+qts = g_new(QTestState, 1);
+qts->wstatus = 0;
+for (int i = 0; i < MAX_IRQ; i++) {
+qts->irq_level[i] = false;
+}
+qts->big_endian = qtest_query_target_endianness(qts);
+
+return qts;
+}
+#endif
QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
{
@@ -379,9 +399,18 @@ static void socket_sendf(int fd, const char *fmt, va_list
ap)
{
gchar *str = g_strdup_vprintf(fmt, ap);
size_t size = strlen(str);
+#ifdef CONFIG_FUZZ
+// Directly call qtest_process_inbuf in the qtest server
+GString *gstr = g_string_new_len(str, size);
+ /* printf(">>> %s",gstr->str); */
+qtest_server_recv(gstr);
+g_string_free(gstr, true);
+g_free(str);
+#else
socket_send(fd, str, size);
g_free(str);
+#endif
}
static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
@@ -433,6 +462,12 @@ static GString *qtest_recv_line(QTestState *s)
size_t offset;
char *eol;
+#ifdef CONFIG_FUZZ
+eol = strchr(recv_str->str, '\n');
+offset = eol - recv_str->str;
+line = g_string_new_len(recv_str->str, offset);
+g_string_erase(recv_str, 0, offset + 1);
+#else
while ((eol = strchr(s->rx->str, '\n')) == NULL) {
ssize_t len;
char buffer[1024];
@@ -453,7 +488,7 @@ static GString *qtest_recv_line(QTestState *s)
offset = eol - s->rx->str;
line = g_string_new_len(s->rx->str, offset);
g_string_erase(s->rx, 0, offset + 1);
-
+#endif
return line;
}
@@ -797,6 +832,9 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
const char *qtest_get_arch(void)
{
+#ifdef CONFIG_FUZZ
+return "i386";
+#endif
const char *qemu = qtest_qemu_binary();
const char *end = strrchr(qemu, '/');
@@ -1339,3 +1377,16 @@ void qmp_assert_error_class(QDict *rsp, const char
*class)
qobject_unref(rsp);
}
+#ifdef CONFIG_FUZZ
+void qtest_clear_rxbuf(QTestState *s){
+g_string_set_size(recv_str,0);
+}
+
+void qtest_client_recv(const char *str, size_t len)
+{
+if(!recv_str)
+recv_str = g_string_new(NULL);
+g_string_append_len(recv_str, str, len);
+return;
+}
+#endif
diff --git a/tests/libqtest.h b/tests/libqtest.h
index cadf1d4a03..dca8f2c2f2 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -1001,4 +1001,10 @@ void qmp_assert_error_class(QDict *rsp, const char
*class);
*/
bool qtest_probe_child(QTestState *s);
+#ifdef CONFIG_FUZZ
+QTestState *qtest_init_fuzz(const char *extra_args, int *sock_fd);
+void qtest_clear_rxbuf(QTestState *s);
+void qtest_client_recv(const char *str, size_t len);
+#endif
+
#endif
--
2.20.1
