* [PATCH for-4.5] xl: fix two memory leaks
@ 2014-11-30 21:54 Wei Liu
2014-12-01 9:34 ` Ian Campbell
0 siblings, 1 reply; 4+ messages in thread
From: Wei Liu @ 2014-11-30 21:54 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Wei Liu, Ian Campbell
There are two invocations of libxl_basename, which returns a malloc'ed
string. Those strings should be freed after used.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/xl_cmdimpl.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9afef3f..716a865 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -920,6 +920,7 @@ static void parse_config_data(const char *config_source,
int pci_permissive = 0;
int pci_seize = 0;
int i, e;
+ const char *basename;
libxl_domain_create_info *c_info = &d_config->c_info;
libxl_domain_build_info *b_info = &d_config->b_info;
@@ -1116,13 +1117,16 @@ static void parse_config_data(const char *config_source,
switch(b_info->type) {
case LIBXL_DOMAIN_TYPE_HVM:
- if (!strcmp(libxl_basename(b_info->kernel), "hvmloader")) {
+ basename = libxl_basename(b_info->kernel);
+ if (!strcmp(basename, "hvmloader")) {
fprintf(stderr, "WARNING: you seem to be using \"kernel\" "
"directive to override HVM guest firmware. Ignore "
"that. Use \"firmware_override\" instead if you "
"really want a non-default firmware\n");
b_info->kernel = NULL;
}
+ free((void*)basename);
+ basename = NULL;
xlu_cfg_replace_string (config, "firmware_override",
&b_info->u.hvm.firmware, 0);
@@ -7021,7 +7025,7 @@ int main_cpupoolcreate(int argc, char **argv)
int config_len = 0;
XLU_Config *config;
const char *buf;
- const char *name;
+ const char *name = NULL;
uint32_t poolid;
libxl_scheduler sched = 0;
XLU_ConfigList *cpus;
@@ -7196,6 +7200,7 @@ out_cfg:
xlu_cfg_destroy(config);
out:
free(config_data);
+ free((void*)name);
return rc;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.5] xl: fix two memory leaks
2014-11-30 21:54 [PATCH for-4.5] xl: fix two memory leaks Wei Liu
@ 2014-12-01 9:34 ` Ian Campbell
2014-12-01 10:10 ` Wei Liu
0 siblings, 1 reply; 4+ messages in thread
From: Ian Campbell @ 2014-12-01 9:34 UTC (permalink / raw)
To: Wei Liu; +Cc: Ian Jackson, xen-devel
On Sun, 2014-11-30 at 21:54 +0000, Wei Liu wrote:
> There are two invocations of libxl_basename, which returns a malloc'ed
> string. Those strings should be freed after used.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
> tools/libxl/xl_cmdimpl.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 9afef3f..716a865 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -920,6 +920,7 @@ static void parse_config_data(const char *config_source,
> int pci_permissive = 0;
> int pci_seize = 0;
> int i, e;
> + const char *basename;
>
> libxl_domain_create_info *c_info = &d_config->c_info;
> libxl_domain_build_info *b_info = &d_config->b_info;
> @@ -1116,13 +1117,16 @@ static void parse_config_data(const char *config_source,
>
> switch(b_info->type) {
> case LIBXL_DOMAIN_TYPE_HVM:
> - if (!strcmp(libxl_basename(b_info->kernel), "hvmloader")) {
> + basename = libxl_basename(b_info->kernel);
> + if (!strcmp(basename, "hvmloader")) {
> fprintf(stderr, "WARNING: you seem to be using \"kernel\" "
> "directive to override HVM guest firmware. Ignore "
> "that. Use \"firmware_override\" instead if you "
> "really want a non-default firmware\n");
> b_info->kernel = NULL;
> }
> + free((void*)basename);
I think you should un-const the declaration (in both cases) rather than
adding casts.
> + basename = NULL;
>
> xlu_cfg_replace_string (config, "firmware_override",
> &b_info->u.hvm.firmware, 0);
> @@ -7021,7 +7025,7 @@ int main_cpupoolcreate(int argc, char **argv)
> int config_len = 0;
> XLU_Config *config;
> const char *buf;
> - const char *name;
> + const char *name = NULL;
> uint32_t poolid;
> libxl_scheduler sched = 0;
> XLU_ConfigList *cpus;
> @@ -7196,6 +7200,7 @@ out_cfg:
> xlu_cfg_destroy(config);
> out:
> free(config_data);
> + free((void*)name);
> return rc;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.5] xl: fix two memory leaks
2014-12-01 9:34 ` Ian Campbell
@ 2014-12-01 10:10 ` Wei Liu
2014-12-01 10:21 ` Ian Campbell
0 siblings, 1 reply; 4+ messages in thread
From: Wei Liu @ 2014-12-01 10:10 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian Jackson, Wei Liu, xen-devel
On Mon, Dec 01, 2014 at 09:34:47AM +0000, Ian Campbell wrote:
> On Sun, 2014-11-30 at 21:54 +0000, Wei Liu wrote:
> > There are two invocations of libxl_basename, which returns a malloc'ed
> > string. Those strings should be freed after used.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > ---
> > tools/libxl/xl_cmdimpl.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 9afef3f..716a865 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -920,6 +920,7 @@ static void parse_config_data(const char *config_source,
> > int pci_permissive = 0;
> > int pci_seize = 0;
> > int i, e;
> > + const char *basename;
> >
> > libxl_domain_create_info *c_info = &d_config->c_info;
> > libxl_domain_build_info *b_info = &d_config->b_info;
> > @@ -1116,13 +1117,16 @@ static void parse_config_data(const char *config_source,
> >
> > switch(b_info->type) {
> > case LIBXL_DOMAIN_TYPE_HVM:
> > - if (!strcmp(libxl_basename(b_info->kernel), "hvmloader")) {
> > + basename = libxl_basename(b_info->kernel);
> > + if (!strcmp(basename, "hvmloader")) {
> > fprintf(stderr, "WARNING: you seem to be using \"kernel\" "
> > "directive to override HVM guest firmware. Ignore "
> > "that. Use \"firmware_override\" instead if you "
> > "really want a non-default firmware\n");
> > b_info->kernel = NULL;
> > }
> > + free((void*)basename);
>
> I think you should un-const the declaration (in both cases) rather than
> adding casts.
>
Is it OK to change the declaration of a public API?
Wei.
> > + basename = NULL;
> >
> > xlu_cfg_replace_string (config, "firmware_override",
> > &b_info->u.hvm.firmware, 0);
> > @@ -7021,7 +7025,7 @@ int main_cpupoolcreate(int argc, char **argv)
> > int config_len = 0;
> > XLU_Config *config;
> > const char *buf;
> > - const char *name;
> > + const char *name = NULL;
> > uint32_t poolid;
> > libxl_scheduler sched = 0;
> > XLU_ConfigList *cpus;
> > @@ -7196,6 +7200,7 @@ out_cfg:
> > xlu_cfg_destroy(config);
> > out:
> > free(config_data);
> > + free((void*)name);
> > return rc;
> > }
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-4.5] xl: fix two memory leaks
2014-12-01 10:10 ` Wei Liu
@ 2014-12-01 10:21 ` Ian Campbell
0 siblings, 0 replies; 4+ messages in thread
From: Ian Campbell @ 2014-12-01 10:21 UTC (permalink / raw)
To: Wei Liu; +Cc: Ian Jackson, xen-devel
On Mon, 2014-12-01 at 10:10 +0000, Wei Liu wrote:
> On Mon, Dec 01, 2014 at 09:34:47AM +0000, Ian Campbell wrote:
> > On Sun, 2014-11-30 at 21:54 +0000, Wei Liu wrote:
> > > There are two invocations of libxl_basename, which returns a malloc'ed
> > > string. Those strings should be freed after used.
> > >
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > ---
> > > tools/libxl/xl_cmdimpl.c | 9 +++++++--
> > > 1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > > index 9afef3f..716a865 100644
> > > --- a/tools/libxl/xl_cmdimpl.c
> > > +++ b/tools/libxl/xl_cmdimpl.c
> > > @@ -920,6 +920,7 @@ static void parse_config_data(const char *config_source,
> > > int pci_permissive = 0;
> > > int pci_seize = 0;
> > > int i, e;
> > > + const char *basename;
> > >
> > > libxl_domain_create_info *c_info = &d_config->c_info;
> > > libxl_domain_build_info *b_info = &d_config->b_info;
> > > @@ -1116,13 +1117,16 @@ static void parse_config_data(const char *config_source,
> > >
> > > switch(b_info->type) {
> > > case LIBXL_DOMAIN_TYPE_HVM:
> > > - if (!strcmp(libxl_basename(b_info->kernel), "hvmloader")) {
> > > + basename = libxl_basename(b_info->kernel);
> > > + if (!strcmp(basename, "hvmloader")) {
> > > fprintf(stderr, "WARNING: you seem to be using \"kernel\" "
> > > "directive to override HVM guest firmware. Ignore "
> > > "that. Use \"firmware_override\" instead if you "
> > > "really want a non-default firmware\n");
> > > b_info->kernel = NULL;
> > > }
> > > + free((void*)basename);
> >
> > I think you should un-const the declaration (in both cases) rather than
> > adding casts.
> >
>
> Is it OK to change the declaration of a public API?
Oh god, libxl_basename returns a dynamically allocated string as a
const. That's a bit mad... At least it is clearly documented as being a
strdup'd thing!
I think we may need to use LIBXL_API_VERSION here to conditionally
include the const. We've done that before see
LIBXL_HAVE_NONCONST_EVENT_OCCURS_EVENT_ARG.
Ian.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-01 10:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-30 21:54 [PATCH for-4.5] xl: fix two memory leaks Wei Liu
2014-12-01 9:34 ` Ian Campbell
2014-12-01 10:10 ` Wei Liu
2014-12-01 10:21 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.