Re: [PATCH v2 2/2] qtest: don't close listener socket in qtest_socket_accept

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The listener FD passed into qtest_socket_accept should be
> considered owned by the caller, and thus is is inappropriate
> and unusual to close it in the method.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qtest/libqtest.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 50487739ef9..0f701679745 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -134,7 +134,6 @@ static int qtest_socket_accept(QTestState *s, int sock)
>                     (void *)&timeout, sizeof(timeout))) {
>          fprintf(stderr, "%s failed to set SO_RCVTIMEO: %s\n",
>                  __func__, strerror(errno));
> -        close(sock);
>          return -1;
>      }
>  
> @@ -142,7 +141,7 @@ static int qtest_socket_accept(QTestState *s, int sock)
>          if (!qtest_probe_child(s)) {
>              fprintf(stderr,
>                      "child process unexpectedly exited, skipping socket accept\n");
> -            goto cleanup;
> +            return -1;

This removes the need for initializing @ret.  Observation, not a demand.

>          }
>  
>          addrlen = sizeof(addr);
> @@ -156,9 +155,6 @@ static int qtest_socket_accept(QTestState *s, int sock)
>          fprintf(stderr, "%s failed: %s\n", __func__, strerror(errno));
>      }
>  
> - cleanup:
> -    close(sock);
> -
>      return ret;
>  }
>  
> @@ -566,6 +562,8 @@ void qtest_connect(QTestState *s)
>      unlink(socket_path);
>      unlink(qmp_socket_path);
>      g_assert(s->fd >= 0 && s->qmp_fd >= 0);
> +    close(s->sock);
> +    close(s->qmpsock);
>      s->sock = s->qmpsock = -1;
>      /* ask endianness of the target */
>      s->big_endian = qtest_query_target_endianness(s);
> @@ -675,6 +673,7 @@ QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
>  
>      *sock_fd = qtest_socket_accept(qts, sock_fd_init);
>  
> +    close(sock_fd_init);
>      unlink(sock_path);
>      g_free(sock_path);
>      rmdir(sock_dir);

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