Hi Oleksandr,
On Wed, Feb 16, 2022 at 08:33:25AM +0200, Oleksandr Andrushchenko wrote:
> diff --git a/tools/libs/vchan/init.c b/tools/libs/vchan/init.c
> index c8510e6ce98a..ae9a6b579753 100644
> --- a/tools/libs/vchan/init.c
> +++ b/tools/libs/vchan/init.c
> @@ -251,6 +253,12 @@ static int init_xs_srv(struct libxenvchan *ctrl, int
> domain, const char* xs_base
> char ref[16];
> char* domid_str = NULL;
> xs_transaction_t xs_trans = XBT_NULL;
> +
> + // store the base path so we can clean up on server closure
> + ctrl->xs_path = strdup(xs_base);
You could do this in libxenvchan_server_init(), this might make it
easier to avoid leaking the path.
You need to initialise ctrl->xs_path in libxenvchan_server_init() in
any case before libxenvchan_close() is called.
I think you need to initialise ctrl->xs_path to NULL in
libxenvchan_client_init() as well even if it's not going to be used, to
avoid issue later in case the field start to be used.
> @@ -298,6 +306,23 @@ retry_transaction:
> return ret;
> }
>
> +void close_xs_srv(struct libxenvchan *ctrl)
> +{
> + struct xs_handle *xs;
> +
> + if (!ctrl->xs_path)
> + return;
> +
> + xs = xs_open(0);
There is missing a xs_close() call.
> + if (!xs)
> + goto fail;
> +
> + xs_rm(xs, XBT_NULL, ctrl->xs_path);
> +
> +fail:
> + free(ctrl->xs_path);
> +}
> +
> static int min_order(size_t siz)
> {
> int rv = PAGE_SHIFT;
> diff --git a/tools/libs/vchan/io.c b/tools/libs/vchan/io.c
> index da303fbc01ca..1f201ad554f2 100644
> --- a/tools/libs/vchan/io.c
> +++ b/tools/libs/vchan/io.c
> @@ -40,6 +40,8 @@
> #include <xenctrl.h>
> #include <libxenvchan.h>
>
> +#include "vchan.h"
> +
> #ifndef PAGE_SHIFT
> #define PAGE_SHIFT 12
> #endif
> @@ -384,5 +386,7 @@ void libxenvchan_close(struct libxenvchan *ctrl)
> if (ctrl->gnttab)
> xengnttab_close(ctrl->gnttab);
> }
> + if (ctrl->is_server)
> + close_xs_srv(ctrl);
Since init_xs_srv() is one of the last step of the initialisation of
*ctrl, I think close_xs_srv() should be one of the first step in
libxenvchan_close().
Thanks,
--
Anthony PERARD
