On Wed, Oct 30, 2019 at 02:49:58PM +0000, Oleinik, Alexander wrote:
> From: Alexander Oleinik <alxndr@bu.edu>
> 
> Signed-off-by: Alexander Oleinik <alxndr@bu.edu>
> ---
> There's a particularily ugly line here:
> qtest_client_set_tx_handler(qts,
>         (void (*)(QTestState *s, const char*, size_t)) send);

Please typedef the function pointer to avoid repetition:

  typedef void (*QTestSendFn)(QTestState *s, const char *buf, size_t len);

And then introduce a wrapper function for type-safety:

  /* A type-safe wrapper for s->send() */
  static void send_wrapper(QTestState *s, const char *buf, size_t len)
  {
      s->send(s, buf, len);
  }

  ...

  qts->send = send;
  qtest_client_set_tx_handler(qts, send_wrapper);

Does this solve the issue?

By the way, I also wonder whether the size_t len arguments are necessary
since const char *buf is a NUL-terminated C string.  The string should
be enough since the length can be calculated from it.

> diff --git a/qtest.c b/qtest.c
> index 9fbfa0f08f..f817a5d789 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -812,6 +812,6 @@ void qtest_server_inproc_recv(void *dummy, const char *buf, size_t size)
>      g_string_append(gstr, buf);
>      if (gstr->str[gstr->len - 1] == '\n') {
>          qtest_process_inbuf(NULL, gstr);
> -        g_string_free(gstr, true);
> +        g_string_truncate(gstr, 0);

Ah, a fix for the bug in an earlier commit.  Please squash it.

> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index ff3153daf2..6143af33da 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -71,6 +71,7 @@ static void qtest_client_set_tx_handler(QTestState *s,
>  static void qtest_client_set_rx_handler(QTestState *s,
>          GString * (*recv)(QTestState *));
>  
> +static GString *recv_str;

Can this be a QTestState field?