On Mon, Jul 10, 2017 at 11:48:27AM +0200, Jens Freimann wrote:
> >@@ -668,7 +668,7 @@ rte_vhost_driver_register(const char *path, uint64_t
> >flags)
> > }
> >
> > vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
> >-
> >+ goto out;
> >out_mutex:
> > if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
> > RTE_LOG(ERR, VHOST_CONFIG,
>
> Thanks for fixing this!
>
> Sorry for introducing this bug, I was about to send this before I saw
> your fix:
>
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 57b86c0..b2158a7 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -668,6 +668,9 @@ rte_vhost_driver_register(const char *path, uint64_t
> flags)
> }
>
> vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
> +out:
> + pthread_mutex_unlock(&vhost_user.mutex);
> + return ret;
>
> out_mutex:
> if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
> @@ -677,9 +680,7 @@ rte_vhost_driver_register(const char *path, uint64_t
> flags)
> out_free:
> free(vsocket->path);
> free(vsocket);
> -out:
> pthread_mutex_unlock(&vhost_user.mutex);
> -
> return ret;
> }
>
> Both works fine, so I leave it up to the maintainers how to fix.
>
> Reviewed-by: Jens Freimann <jfreim...@redhat.com>
I actually prefers below: I don't want a simple "out" on top of "out_mutex"
and "out_free", and an unconditional "goto" looks weird to me.
---
@@ -669,6 +669,9 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
+ pthread_mutex_unlock(&vhost_user.mutex);
+ return ret;
+
out_mutex:
if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
RTE_LOG(ERR, VHOST_CONFIG,
Applied to dpdk-next-virtio, with above changes.
Thanks!
--yliu
