[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] tools: remove xenstore entries on vchan server closure
On Wed, Feb 16, 2022 at 1:33 AM Oleksandr Andrushchenko <andr2000@xxxxxxxxx> wrote: > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx> > > vchan server creates XenStore entries to advertise its event channel and > ring, but those are not removed after the server quits. > Add additional cleanup step, so those are removed, so clients do not try > to connect to a non-existing server. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx> > > --- > Since v1: > - add NULL check after strdup > --- > tools/include/libxenvchan.h | 5 +++++ > tools/libs/vchan/init.c | 25 +++++++++++++++++++++++++ > tools/libs/vchan/io.c | 4 ++++ > tools/libs/vchan/vchan.h | 31 +++++++++++++++++++++++++++++++ > 4 files changed, 65 insertions(+) > create mode 100644 tools/libs/vchan/vchan.h > 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 > @@ -46,6 +46,8 @@ > #include <xen/sys/gntdev.h> > #include <libxenvchan.h> > > +#include "vchan.h" > + > #ifndef PAGE_SHIFT > #define PAGE_SHIFT 12 > #endif > @@ -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); > + if (!ctrl->xs_path) > + goto fail; > + > xs = xs_open(0); > if (!xs) > goto fail; Hi, Oleksandr, You now have multiple conditions goto fail, so I think you need to add the below snippet to avoid leaking memory. if (ret) { free(ctrl->xs_path) ctrl->xs_path = NULL; } It's actually okay with your patch since libxenvchan_server_init() does: if (init_xs_srv(ctrl, domain, xs_path, ring_ref)) goto out; return ctrl; out: libxenvchan_close(ctrl); return 0; and libxenvchan_close() will free xs_path. But it's a little more robust to cleanup local to the failure. Thanks, Jason
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |