Re: [Xen-devel] [PATCH v3 01/10] xen: make xen loader callable multiple times

2016-02-18 Thread Juergen Gross
On 18/02/16 17:58, Daniel Kiper wrote:
> On Thu, Feb 18, 2016 at 11:32:16AM +0100, Juergen Gross wrote:
>> On 18/02/16 11:12, Daniel Kiper wrote:
>>> On Wed, Feb 17, 2016 at 06:19:28PM +0100, Juergen Gross wrote:
 The loader for xen paravirtualized environment isn't callable multiple
 times as it won't free any memory in case of failure.

 Call grub_relocator_unload() as other modules do it before allocating
>>>
>>> Do you mean grub_xen_reset?
>>
>> No. I do want to call grub_relocator_unload() and I'm doing it in
>> grub_xen_reset(). Other modules don't call grub_xen_reset(). :-)
>>
>>>
 a new relocator or when unloading the module.

 Signed-off-by: Juergen Gross 
 ---
  grub-core/loader/i386/xen.c| 28 +++-
  grub-core/loader/i386/xen_fileXX.c | 17 +++--
  2 files changed, 30 insertions(+), 15 deletions(-)

 diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
 index c4d9689..ff7c553 100644
 --- a/grub-core/loader/i386/xen.c
 +++ b/grub-core/loader/i386/xen.c
 @@ -316,11 +316,23 @@ grub_xen_boot (void)
  xen_inf.virt_base);
  }

 +static void
 +grub_xen_reset (void)
 +{
 +  grub_memset (_start, 0, sizeof (next_start));
 +  xen_module_info_page = NULL;
 +  n_modules = 0;
 +
 +  grub_relocator_unload (relocator);
 +  relocator = NULL;
 +  loaded = 0;
 +}
 +
  static grub_err_t
  grub_xen_unload (void)
  {
 +  grub_xen_reset ();
grub_dl_unref (my_mod);
 -  loaded = 0;
return GRUB_ERR_NONE;
  }

 @@ -403,10 +415,7 @@ grub_cmd_xen (grub_command_t cmd __attribute__ 
 ((unused)),

grub_loader_unset ();

 -  grub_memset (_start, 0, sizeof (next_start));
 -
 -  xen_module_info_page = NULL;
 -  n_modules = 0;
 +  grub_xen_reset ();

grub_create_loader_cmdline (argc - 1, argv + 1,
  (char *) next_start.cmd_line,
 @@ -503,16 +512,17 @@ grub_cmd_xen (grub_command_t cmd __attribute__ 
 ((unused)),
goto fail;

  fail:
 +  err = grub_errno;
>>>
>>> I do not think this is needed.
>>
>> grub_elf_close() and others might clobber grub_errno.
> 
> OK, so, please say it in comment. It is not obvious.

Okay.

if (elf)
  grub_elf_close (elf);
else if (file)
  grub_file_close (file);

 -  if (grub_errno != GRUB_ERR_NONE)
 -loaded = 0;
 +  if (err != GRUB_ERR_NONE)
 +grub_xen_reset ();

 -  return grub_errno;
 +  return err;
  }

  static grub_err_t
 @@ -552,7 +562,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
 ((unused)),
  {
err = grub_relocator_alloc_chunk_addr (relocator, , max_addr, 
 size);
if (err)
 -  return err;
 +  goto fail;
>>>
>>> It looks that this change should not be part of this patch.
>>
>> Why not? It's correcting a memory leak in case of failure. Like the
>> other cases below, too. That's the purpose of this patch, after all.
> 
> OK but you are referring to grub_relocator_unload() in commit message
> and below you are trying to fix different memory leak in the same patch.
> So, I think everything below should be separate patch.

Okay.


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 01/10] xen: make xen loader callable multiple times

2016-02-18 Thread Daniel Kiper
On Thu, Feb 18, 2016 at 11:32:16AM +0100, Juergen Gross wrote:
> On 18/02/16 11:12, Daniel Kiper wrote:
> > On Wed, Feb 17, 2016 at 06:19:28PM +0100, Juergen Gross wrote:
> >> The loader for xen paravirtualized environment isn't callable multiple
> >> times as it won't free any memory in case of failure.
> >>
> >> Call grub_relocator_unload() as other modules do it before allocating
> >
> > Do you mean grub_xen_reset?
>
> No. I do want to call grub_relocator_unload() and I'm doing it in
> grub_xen_reset(). Other modules don't call grub_xen_reset(). :-)
>
> >
> >> a new relocator or when unloading the module.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >>  grub-core/loader/i386/xen.c| 28 +++-
> >>  grub-core/loader/i386/xen_fileXX.c | 17 +++--
> >>  2 files changed, 30 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> >> index c4d9689..ff7c553 100644
> >> --- a/grub-core/loader/i386/xen.c
> >> +++ b/grub-core/loader/i386/xen.c
> >> @@ -316,11 +316,23 @@ grub_xen_boot (void)
> >>  xen_inf.virt_base);
> >>  }
> >>
> >> +static void
> >> +grub_xen_reset (void)
> >> +{
> >> +  grub_memset (_start, 0, sizeof (next_start));
> >> +  xen_module_info_page = NULL;
> >> +  n_modules = 0;
> >> +
> >> +  grub_relocator_unload (relocator);
> >> +  relocator = NULL;
> >> +  loaded = 0;
> >> +}
> >> +
> >>  static grub_err_t
> >>  grub_xen_unload (void)
> >>  {
> >> +  grub_xen_reset ();
> >>grub_dl_unref (my_mod);
> >> -  loaded = 0;
> >>return GRUB_ERR_NONE;
> >>  }
> >>
> >> @@ -403,10 +415,7 @@ grub_cmd_xen (grub_command_t cmd __attribute__ 
> >> ((unused)),
> >>
> >>grub_loader_unset ();
> >>
> >> -  grub_memset (_start, 0, sizeof (next_start));
> >> -
> >> -  xen_module_info_page = NULL;
> >> -  n_modules = 0;
> >> +  grub_xen_reset ();
> >>
> >>grub_create_loader_cmdline (argc - 1, argv + 1,
> >>  (char *) next_start.cmd_line,
> >> @@ -503,16 +512,17 @@ grub_cmd_xen (grub_command_t cmd __attribute__ 
> >> ((unused)),
> >>goto fail;
> >>
> >>  fail:
> >> +  err = grub_errno;
> >
> > I do not think this is needed.
>
> grub_elf_close() and others might clobber grub_errno.

OK, so, please say it in comment. It is not obvious.

> >>if (elf)
> >>  grub_elf_close (elf);
> >>else if (file)
> >>  grub_file_close (file);
> >>
> >> -  if (grub_errno != GRUB_ERR_NONE)
> >> -loaded = 0;
> >> +  if (err != GRUB_ERR_NONE)
> >> +grub_xen_reset ();
> >>
> >> -  return grub_errno;
> >> +  return err;
> >>  }
> >>
> >>  static grub_err_t
> >> @@ -552,7 +562,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> >> ((unused)),
> >>  {
> >>err = grub_relocator_alloc_chunk_addr (relocator, , max_addr, 
> >> size);
> >>if (err)
> >> -  return err;
> >> +  goto fail;
> >
> > It looks that this change should not be part of this patch.
>
> Why not? It's correcting a memory leak in case of failure. Like the
> other cases below, too. That's the purpose of this patch, after all.

OK but you are referring to grub_relocator_unload() in commit message
and below you are trying to fix different memory leak in the same patch.
So, I think everything below should be separate patch.

Daniel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v3 01/10] xen: make xen loader callable multiple times

2016-02-18 Thread Juergen Gross
On 18/02/16 11:12, Daniel Kiper wrote:
> On Wed, Feb 17, 2016 at 06:19:28PM +0100, Juergen Gross wrote:
>> The loader for xen paravirtualized environment isn't callable multiple
>> times as it won't free any memory in case of failure.
>>
>> Call grub_relocator_unload() as other modules do it before allocating
> 
> Do you mean grub_xen_reset?

No. I do want to call grub_relocator_unload() and I'm doing it in
grub_xen_reset(). Other modules don't call grub_xen_reset(). :-)

> 
>> a new relocator or when unloading the module.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>  grub-core/loader/i386/xen.c| 28 +++-
>>  grub-core/loader/i386/xen_fileXX.c | 17 +++--
>>  2 files changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>> index c4d9689..ff7c553 100644
>> --- a/grub-core/loader/i386/xen.c
>> +++ b/grub-core/loader/i386/xen.c
>> @@ -316,11 +316,23 @@ grub_xen_boot (void)
>>xen_inf.virt_base);
>>  }
>>
>> +static void
>> +grub_xen_reset (void)
>> +{
>> +  grub_memset (_start, 0, sizeof (next_start));
>> +  xen_module_info_page = NULL;
>> +  n_modules = 0;
>> +
>> +  grub_relocator_unload (relocator);
>> +  relocator = NULL;
>> +  loaded = 0;
>> +}
>> +
>>  static grub_err_t
>>  grub_xen_unload (void)
>>  {
>> +  grub_xen_reset ();
>>grub_dl_unref (my_mod);
>> -  loaded = 0;
>>return GRUB_ERR_NONE;
>>  }
>>
>> @@ -403,10 +415,7 @@ grub_cmd_xen (grub_command_t cmd __attribute__ 
>> ((unused)),
>>
>>grub_loader_unset ();
>>
>> -  grub_memset (_start, 0, sizeof (next_start));
>> -
>> -  xen_module_info_page = NULL;
>> -  n_modules = 0;
>> +  grub_xen_reset ();
>>
>>grub_create_loader_cmdline (argc - 1, argv + 1,
>>(char *) next_start.cmd_line,
>> @@ -503,16 +512,17 @@ grub_cmd_xen (grub_command_t cmd __attribute__ 
>> ((unused)),
>>goto fail;
>>
>>  fail:
>> +  err = grub_errno;
> 
> I do not think this is needed.

grub_elf_close() and others might clobber grub_errno.

>>if (elf)
>>  grub_elf_close (elf);
>>else if (file)
>>  grub_file_close (file);
>>
>> -  if (grub_errno != GRUB_ERR_NONE)
>> -loaded = 0;
>> +  if (err != GRUB_ERR_NONE)
>> +grub_xen_reset ();
>>
>> -  return grub_errno;
>> +  return err;
>>  }
>>
>>  static grub_err_t
>> @@ -552,7 +562,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
>> ((unused)),
>>  {
>>err = grub_relocator_alloc_chunk_addr (relocator, , max_addr, 
>> size);
>>if (err)
>> -return err;
>> +goto fail;
> 
> It looks that this change should not be part of this patch.

Why not? It's correcting a memory leak in case of failure. Like the
other cases below, too. That's the purpose of this patch, after all.


Juergen

> 
>>if (grub_initrd_load (_ctx, argv,
>>  get_virtual_current_address (ch)))
>> diff --git a/grub-core/loader/i386/xen_fileXX.c 
>> b/grub-core/loader/i386/xen_fileXX.c
>> index 1ba5649..5475819 100644
>> --- a/grub-core/loader/i386/xen_fileXX.c
>> +++ b/grub-core/loader/i386/xen_fileXX.c
>> @@ -35,7 +35,8 @@ parse_xen_guest (grub_elf_t elf, struct grub_xen_file_info 
>> *xi,
>>if (grub_file_read (elf->file, buf, sz) != (grub_ssize_t) sz)
>>  {
>>if (grub_errno)
>> -return grub_errno;
>> +goto out;
>> +  grub_free (buf);
> 
> Ditto.
> 
>>return grub_error (GRUB_ERR_BAD_OS, N_("premature end of file %s"),
>>   elf->file->name);
>>  }
>> @@ -123,14 +124,14 @@ parse_xen_guest (grub_elf_t elf, struct 
>> grub_xen_file_info *xi,
>>  {
>>xi->virt_base = grub_strtoull (ptr + sizeof ("VIRT_BASE=") - 1, , 
>> 16);
>>if (grub_errno)
>> -return grub_errno;
>> +goto out;
> 
> Ditto.
> 
>>continue;
>>  }
>>if (grub_strncmp (ptr, "VIRT_ENTRY=", sizeof ("VIRT_ENTRY=") - 1) == 
>> 0)
>>  {
>>xi->entry_point = grub_strtoull (ptr + sizeof ("VIRT_ENTRY=") - 1, 
>> , 16);
>>if (grub_errno)
>> -return grub_errno;
>> +goto out;
> 
> Ditto.
> 
>>continue;
>>  }
>>if (grub_strncmp (ptr, "HYPERCALL_PAGE=", sizeof ("HYPERCALL_PAGE=") 
>> - 1) == 0)
>> @@ -138,7 +139,7 @@ parse_xen_guest (grub_elf_t elf, struct 
>> grub_xen_file_info *xi,
>>xi->hypercall_page = grub_strtoull (ptr + sizeof ("HYPERCALL_PAGE=") 
>> - 1, , 16);
>>xi->has_hypercall_page = 1;
>>if (grub_errno)
>> -return grub_errno;
>> +goto out;
> 
> Ditto.
> 
>>continue;
>>  }
>>if (grub_strncmp (ptr, "ELF_PADDR_OFFSET=", sizeof 
>> ("ELF_PADDR_OFFSET=") - 1) == 0)
>> @@ -146,7 +147,7 @@ parse_xen_guest (grub_elf_t elf, struct 
>> grub_xen_file_info *xi,
>>xi->paddr_offset = grub_strtoull (ptr + sizeof ("ELF_PADDR_OFFSET=") 
>> - 1, , 16);
>>has_paddr = 1;
>>if (grub_errno)
>> -

Re: [Xen-devel] [PATCH v3 01/10] xen: make xen loader callable multiple times

2016-02-18 Thread Daniel Kiper
On Wed, Feb 17, 2016 at 06:19:28PM +0100, Juergen Gross wrote:
> The loader for xen paravirtualized environment isn't callable multiple
> times as it won't free any memory in case of failure.
>
> Call grub_relocator_unload() as other modules do it before allocating

Do you mean grub_xen_reset?

> a new relocator or when unloading the module.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/loader/i386/xen.c| 28 +++-
>  grub-core/loader/i386/xen_fileXX.c | 17 +++--
>  2 files changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> index c4d9689..ff7c553 100644
> --- a/grub-core/loader/i386/xen.c
> +++ b/grub-core/loader/i386/xen.c
> @@ -316,11 +316,23 @@ grub_xen_boot (void)
> xen_inf.virt_base);
>  }
>
> +static void
> +grub_xen_reset (void)
> +{
> +  grub_memset (_start, 0, sizeof (next_start));
> +  xen_module_info_page = NULL;
> +  n_modules = 0;
> +
> +  grub_relocator_unload (relocator);
> +  relocator = NULL;
> +  loaded = 0;
> +}
> +
>  static grub_err_t
>  grub_xen_unload (void)
>  {
> +  grub_xen_reset ();
>grub_dl_unref (my_mod);
> -  loaded = 0;
>return GRUB_ERR_NONE;
>  }
>
> @@ -403,10 +415,7 @@ grub_cmd_xen (grub_command_t cmd __attribute__ 
> ((unused)),
>
>grub_loader_unset ();
>
> -  grub_memset (_start, 0, sizeof (next_start));
> -
> -  xen_module_info_page = NULL;
> -  n_modules = 0;
> +  grub_xen_reset ();
>
>grub_create_loader_cmdline (argc - 1, argv + 1,
> (char *) next_start.cmd_line,
> @@ -503,16 +512,17 @@ grub_cmd_xen (grub_command_t cmd __attribute__ 
> ((unused)),
>goto fail;
>
>  fail:
> +  err = grub_errno;

I do not think this is needed.

>if (elf)
>  grub_elf_close (elf);
>else if (file)
>  grub_file_close (file);
>
> -  if (grub_errno != GRUB_ERR_NONE)
> -loaded = 0;
> +  if (err != GRUB_ERR_NONE)
> +grub_xen_reset ();
>
> -  return grub_errno;
> +  return err;
>  }
>
>  static grub_err_t
> @@ -552,7 +562,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> ((unused)),
>  {
>err = grub_relocator_alloc_chunk_addr (relocator, , max_addr, size);
>if (err)
> - return err;
> + goto fail;

It looks that this change should not be part of this patch.

>if (grub_initrd_load (_ctx, argv,
>   get_virtual_current_address (ch)))
> diff --git a/grub-core/loader/i386/xen_fileXX.c 
> b/grub-core/loader/i386/xen_fileXX.c
> index 1ba5649..5475819 100644
> --- a/grub-core/loader/i386/xen_fileXX.c
> +++ b/grub-core/loader/i386/xen_fileXX.c
> @@ -35,7 +35,8 @@ parse_xen_guest (grub_elf_t elf, struct grub_xen_file_info 
> *xi,
>if (grub_file_read (elf->file, buf, sz) != (grub_ssize_t) sz)
>  {
>if (grub_errno)
> - return grub_errno;
> + goto out;
> +  grub_free (buf);

Ditto.

>return grub_error (GRUB_ERR_BAD_OS, N_("premature end of file %s"),
>elf->file->name);
>  }
> @@ -123,14 +124,14 @@ parse_xen_guest (grub_elf_t elf, struct 
> grub_xen_file_info *xi,
>   {
> xi->virt_base = grub_strtoull (ptr + sizeof ("VIRT_BASE=") - 1, , 
> 16);
> if (grub_errno)
> - return grub_errno;
> + goto out;

Ditto.

> continue;
>   }
>if (grub_strncmp (ptr, "VIRT_ENTRY=", sizeof ("VIRT_ENTRY=") - 1) == 0)
>   {
> xi->entry_point = grub_strtoull (ptr + sizeof ("VIRT_ENTRY=") - 1, 
> , 16);
> if (grub_errno)
> - return grub_errno;
> + goto out;

Ditto.

> continue;
>   }
>if (grub_strncmp (ptr, "HYPERCALL_PAGE=", sizeof ("HYPERCALL_PAGE=") - 
> 1) == 0)
> @@ -138,7 +139,7 @@ parse_xen_guest (grub_elf_t elf, struct 
> grub_xen_file_info *xi,
> xi->hypercall_page = grub_strtoull (ptr + sizeof ("HYPERCALL_PAGE=") 
> - 1, , 16);
> xi->has_hypercall_page = 1;
> if (grub_errno)
> - return grub_errno;
> + goto out;

Ditto.

> continue;
>   }
>if (grub_strncmp (ptr, "ELF_PADDR_OFFSET=", sizeof 
> ("ELF_PADDR_OFFSET=") - 1) == 0)
> @@ -146,7 +147,7 @@ parse_xen_guest (grub_elf_t elf, struct 
> grub_xen_file_info *xi,
> xi->paddr_offset = grub_strtoull (ptr + sizeof ("ELF_PADDR_OFFSET=") 
> - 1, , 16);
> has_paddr = 1;
> if (grub_errno)
> - return grub_errno;
> + goto out;

Ditto.

> continue;
>   }
>  }
> @@ -154,7 +155,11 @@ parse_xen_guest (grub_elf_t elf, struct 
> grub_xen_file_info *xi,
>  xi->hypercall_page = (xi->hypercall_page << 12) + xi->virt_base;
>if (!has_paddr)
>  xi->paddr_offset = xi->virt_base;
> -  return GRUB_ERR_NONE;
> +
> +out:
> +  grub_free (buf);
> +
> +  return grub_errno;

Make sense but this should be separate patch.

Daniel

___
Xen-devel mailing list

[Xen-devel] [PATCH v3 01/10] xen: make xen loader callable multiple times

2016-02-17 Thread Juergen Gross
The loader for xen paravirtualized environment isn't callable multiple
times as it won't free any memory in case of failure.

Call grub_relocator_unload() as other modules do it before allocating
a new relocator or when unloading the module.

Signed-off-by: Juergen Gross 
---
 grub-core/loader/i386/xen.c| 28 +++-
 grub-core/loader/i386/xen_fileXX.c | 17 +++--
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index c4d9689..ff7c553 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -316,11 +316,23 @@ grub_xen_boot (void)
  xen_inf.virt_base);
 }
 
+static void
+grub_xen_reset (void)
+{
+  grub_memset (_start, 0, sizeof (next_start));
+  xen_module_info_page = NULL;
+  n_modules = 0;
+
+  grub_relocator_unload (relocator);
+  relocator = NULL;
+  loaded = 0;
+}
+
 static grub_err_t
 grub_xen_unload (void)
 {
+  grub_xen_reset ();
   grub_dl_unref (my_mod);
-  loaded = 0;
   return GRUB_ERR_NONE;
 }
 
@@ -403,10 +415,7 @@ grub_cmd_xen (grub_command_t cmd __attribute__ ((unused)),
 
   grub_loader_unset ();
 
-  grub_memset (_start, 0, sizeof (next_start));
-
-  xen_module_info_page = NULL;
-  n_modules = 0;
+  grub_xen_reset ();
 
   grub_create_loader_cmdline (argc - 1, argv + 1,
  (char *) next_start.cmd_line,
@@ -503,16 +512,17 @@ grub_cmd_xen (grub_command_t cmd __attribute__ ((unused)),
   goto fail;
 
 fail:
+  err = grub_errno;
 
   if (elf)
 grub_elf_close (elf);
   else if (file)
 grub_file_close (file);
 
-  if (grub_errno != GRUB_ERR_NONE)
-loaded = 0;
+  if (err != GRUB_ERR_NONE)
+grub_xen_reset ();
 
-  return grub_errno;
+  return err;
 }
 
 static grub_err_t
@@ -552,7 +562,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
((unused)),
 {
   err = grub_relocator_alloc_chunk_addr (relocator, , max_addr, size);
   if (err)
-   return err;
+   goto fail;
 
   if (grub_initrd_load (_ctx, argv,
get_virtual_current_address (ch)))
diff --git a/grub-core/loader/i386/xen_fileXX.c 
b/grub-core/loader/i386/xen_fileXX.c
index 1ba5649..5475819 100644
--- a/grub-core/loader/i386/xen_fileXX.c
+++ b/grub-core/loader/i386/xen_fileXX.c
@@ -35,7 +35,8 @@ parse_xen_guest (grub_elf_t elf, struct grub_xen_file_info 
*xi,
   if (grub_file_read (elf->file, buf, sz) != (grub_ssize_t) sz)
 {
   if (grub_errno)
-   return grub_errno;
+   goto out;
+  grub_free (buf);
   return grub_error (GRUB_ERR_BAD_OS, N_("premature end of file %s"),
 elf->file->name);
 }
@@ -123,14 +124,14 @@ parse_xen_guest (grub_elf_t elf, struct 
grub_xen_file_info *xi,
{
  xi->virt_base = grub_strtoull (ptr + sizeof ("VIRT_BASE=") - 1, , 
16);
  if (grub_errno)
-   return grub_errno;
+   goto out;
  continue;
}
   if (grub_strncmp (ptr, "VIRT_ENTRY=", sizeof ("VIRT_ENTRY=") - 1) == 0)
{
  xi->entry_point = grub_strtoull (ptr + sizeof ("VIRT_ENTRY=") - 1, 
, 16);
  if (grub_errno)
-   return grub_errno;
+   goto out;
  continue;
}
   if (grub_strncmp (ptr, "HYPERCALL_PAGE=", sizeof ("HYPERCALL_PAGE=") - 
1) == 0)
@@ -138,7 +139,7 @@ parse_xen_guest (grub_elf_t elf, struct grub_xen_file_info 
*xi,
  xi->hypercall_page = grub_strtoull (ptr + sizeof ("HYPERCALL_PAGE=") 
- 1, , 16);
  xi->has_hypercall_page = 1;
  if (grub_errno)
-   return grub_errno;
+   goto out;
  continue;
}
   if (grub_strncmp (ptr, "ELF_PADDR_OFFSET=", sizeof ("ELF_PADDR_OFFSET=") 
- 1) == 0)
@@ -146,7 +147,7 @@ parse_xen_guest (grub_elf_t elf, struct grub_xen_file_info 
*xi,
  xi->paddr_offset = grub_strtoull (ptr + sizeof ("ELF_PADDR_OFFSET=") 
- 1, , 16);
  has_paddr = 1;
  if (grub_errno)
-   return grub_errno;
+   goto out;
  continue;
}
 }
@@ -154,7 +155,11 @@ parse_xen_guest (grub_elf_t elf, struct grub_xen_file_info 
*xi,
 xi->hypercall_page = (xi->hypercall_page << 12) + xi->virt_base;
   if (!has_paddr)
 xi->paddr_offset = xi->virt_base;
-  return GRUB_ERR_NONE;
+
+out:
+  grub_free (buf);
+
+  return grub_errno;
 }
 
 #pragma GCC diagnostic ignored "-Wcast-align"
-- 
2.6.2


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel