On 2018-08-24 at 20:14:37 +0300, Michael S. Tsirkin wrote: > On Fri, Aug 24, 2018 at 05:57:06PM +0100, Peter Maydell wrote: > > On 24 August 2018 at 17:53, Michael S. Tsirkin <m...@redhat.com> wrote: > > > On Fri, Aug 24, 2018 at 04:13:18PM +0100, Peter Maydell wrote: > > >> object_get_canonical_path_component() returns a string which > > >> must be freed using g_free(). > > > > > Like the following? Junyan, could you pls try this one and confirm? > > > > > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > > > > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > > > index 2476dcb435..d88125b86e 100644 > > > --- a/backends/hostmem-file.c > > > +++ b/backends/hostmem-file.c > > > @@ -154,11 +154,14 @@ static void file_memory_backend_set_pmem(Object *o, > > > bool value, Error **errp) > > > #ifndef CONFIG_LIBPMEM > > > if (value) { > > > Error *local_err = NULL; > > > + char *path = object_get_canonical_path_component(o); > > > + > > > error_setg(&local_err, > > > "Lack of libpmem support while setting the 'pmem=on'" > > > " of %s '%s'. We can't ensure data persistence.", > > > object_get_typename(o), > > > - object_get_canonical_path_component(o)); > > > + ); > > > + g_free(path); > > > error_propagate(errp, local_err); > > > return; > > > > Yep, like that (though I would put the closing ");" on > > the line with object_get_typename() and delete the trailing comma). > > > > thanks > > -- PMM >a
Ah.. Thanks Michael/Peter to identify/prepare this fix. > oh i forgot to add in "path". > I didn't build with libpmem installed Maybe you already have libpmem installed yet, it is *ifndef* CONFIG_LIBPMEM ^_^ > Should have been (still untested): > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > index 2476dcb435..72e7055ee7 100644 > --- a/backends/hostmem-file.c > +++ b/backends/hostmem-file.c > @@ -154,11 +154,14 @@ static void file_memory_backend_set_pmem(Object *o, > bool value, Error **errp) > #ifndef CONFIG_LIBPMEM > if (value) { > Error *local_err = NULL; > + char *path = object_get_canonical_path_component(o); > + > error_setg(&local_err, > "Lack of libpmem support while setting the 'pmem=on'" > " of %s '%s'. We can't ensure data persistence.", > object_get_typename(o), > - object_get_canonical_path_component(o)); > + path); > + g_free(path); > error_propagate(errp, local_err); > return; > } > Ah.. I saw another place still have the seem leak, on the previous few lines in file_memory_backend_set_pmem, I will prepare another patch to fix the leaks in these two place. Regards Yi.