[PATCH 2/3] tools/libxl/libxl_cpuid.c: Fix leak of resstr on error path

[PATCH 2/3] tools/libxl/libxl_cpuid.c: Fix leak of resstr on error path

[PRAMOD DEVENDRA]

From: Pramod Devendra <pramod.devendra@citrix.com>

Signed-off-by: Pramod Devendra <pramod.devendra@citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_cpuid.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index b0bdc9d..c66e912 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -223,9 +223,6 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
     }
     entry = cpuid_find_match(cpuid, flag->leaf, flag->subleaf);
     resstr = entry->policy[flag->reg - 1];
-    if (resstr == NULL) {
-        resstr = strdup("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx");
-    }
     num = strtoull(val, &endptr, 0);
     flags[flag->length] = 0;
     if (endptr != val) {
@@ -242,6 +239,11 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
             return 3;
         }
     }
+
+    if (resstr == NULL) {
+        resstr = strdup("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx");
+    }
+
     /* the family and model entry is potentially split up across
      * two fields in Fn0000_0001_EAX, so handle them here separately.
      */
-- 
1.7.10.4

Re: [PATCH 2/3] tools/libxl/libxl_cpuid.c: Fix leak of resstr on error path

[Wei Liu]

On Mon, Mar 16, 2015 at 10:06:17AM +0000, PRAMOD DEVENDRA wrote:
> From: Pramod Devendra <pramod.devendra@citrix.com>
> 
> Signed-off-by: Pramod Devendra <pramod.devendra@citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
>  tools/libxl/libxl_cpuid.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
> index b0bdc9d..c66e912 100644
> --- a/tools/libxl/libxl_cpuid.c
> +++ b/tools/libxl/libxl_cpuid.c
> @@ -223,9 +223,6 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
>      }
>      entry = cpuid_find_match(cpuid, flag->leaf, flag->subleaf);
>      resstr = entry->policy[flag->reg - 1];
> -    if (resstr == NULL) {
> -        resstr = strdup("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx");
> -    }

Minor nit. I would prefer "resstr = " be grouped with the code you
moved. No need to resend though.

Wei.

>      num = strtoull(val, &endptr, 0);
>      flags[flag->length] = 0;
>      if (endptr != val) {
> @@ -242,6 +239,11 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
>              return 3;
>          }
>      }
> +
> +    if (resstr == NULL) {
> +        resstr = strdup("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx");
> +    }
> +
>      /* the family and model entry is potentially split up across
>       * two fields in Fn0000_0001_EAX, so handle them here separately.
>       */
> -- 
> 1.7.10.4

Re: [PATCH 2/3] tools/libxl/libxl_cpuid.c: Fix leak of resstr on error path

[Ian Campbell]

On Tue, 2015-03-17 at 15:44 +0000, Wei Liu wrote:
> On Mon, Mar 16, 2015 at 10:06:17AM +0000, PRAMOD DEVENDRA wrote:
> > From: Pramod Devendra <pramod.devendra@citrix.com>
> > 
> > Signed-off-by: Pramod Devendra <pramod.devendra@citrix.com>
> > CC: Ian Jackson <ian.jackson@eu.citrix.com>
> > CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > CC: Ian Campbell <ian.campbell@citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Applied.

> >      resstr = entry->policy[flag->reg - 1];
> > -    if (resstr == NULL) {
> > -        resstr = strdup("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx");
> > -    }
> 
> Minor nit. I would prefer "resstr = " be grouped with the code you
> moved. No need to resend though.

Do you mean the restr = entry->... bit?